Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v7 0/3] Broadcom FlexRM ring manager support
From: Jassi Brar @ 2017-03-29  5:35 UTC (permalink / raw)
  To: Anup Patel
  Cc: Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
	Rob Rice, Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	BCM Kernel Feedback
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Wed, Mar 29, 2017 at 11:00 AM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> The Broadcom FlexRM ring manager provides producer-consumer style
> ring interface for offload engines on Broadcom iProc SoCs. We can
> have one or more instances of Broadcom FlexRM ring manager in a SoC.
>
> This patchset adds a mailbox driver for Broadcom FlexRM ring manager
> which can be used by offload engine drivers as mailbox clients.
>
> The Broadcom FlexRM mailbox driver is feature complete for RAID and
> Crypto offload engines. We will have incremental patches in-future
> for ring-level statistics using debugfs and minor optimizations.
>
> This patchset is based on Linux-4.11-rc4 and it is also available
> at flexrm-v7 branch of https://github.com/Broadcom/arm64-linux.git
>
> Changes since v6:
>  - Rebased patches for Linux-4.11-rc4
>  - Added a patch to remove depends on COMPILE_TEST for kconfig
>    option BCM_FLEXRM_MBOX
>
You only needed to send the fix to compilation failure reports, not
the whole series.
I believe only the patch-3 is new in the series and I need not even
look at patch-1,2 ?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v7 3/3] mailbox: Remove depends on COMPILE_TEST for BCM_FLEXRM_MBOX
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

The Broadcom FlexRM mailbox driver uses platform MSI support but
not all ARCHs provide asm/msi.h. Due to this, we get compilation
error in Broadcom FlexRM mailbox driver via linux/msi.h on ARCHs
which lack asm/msi.h.

This patch removes "depends on COMPILE_TEST" for Kconfig option
BCM_FLEXRM_MBOX so that Broadcom FlexRM mailbox driver is only
compiled for ARM64.

Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/mailbox/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 305018c..9c19b9f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -155,7 +155,7 @@ config BCM_PDC_MBOX
 
 config BCM_FLEXRM_MBOX
 	tristate "Broadcom FlexRM Mailbox"
-	depends on ARM64 || COMPILE_TEST
+	depends on ARM64
 	depends on HAS_DMA
 	select GENERIC_MSI_IRQ_DOMAIN
 	default ARCH_BCM_IPROC
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v7 2/3] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

This patch adds device tree bindings document for the FlexRM
ring manager found on Broadcom iProc SoCs.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
new file mode 100644
index 0000000..752ae6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
@@ -0,0 +1,59 @@
+Broadcom FlexRM Ring Manager
+============================
+The Broadcom FlexRM ring manager provides a set of rings which can be
+used to submit work to offload engines. An SoC may have multiple FlexRM
+hardware blocks. There is one device tree entry per FlexRM block. The
+FlexRM driver will create a mailbox-controller instance for given FlexRM
+hardware block where each mailbox channel is a separate FlexRM ring.
+
+Required properties:
+--------------------
+- compatible:	Should be "brcm,iproc-flexrm-mbox"
+- reg:		Specifies base physical address and size of the FlexRM
+		ring registers
+- msi-parent:	Phandles (and potential Device IDs) to MSI controllers
+		The FlexRM engine will send MSIs (instead of wired
+		interrupts) to CPU. There is one MSI for each FlexRM ring.
+		Refer devicetree/bindings/interrupt-controller/msi.txt
+- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
+		channel. This should be 3.
+
+		The 1st cell is the mailbox channel number.
+
+		The 2nd cell contains MSI completion threshold. This is the
+		number of completion messages for which FlexRM will inject
+		one MSI interrupt to CPU.
+
+		The 3nd cell contains MSI timer value representing time for
+		which FlexRM will wait to accumulate N completion messages
+		where N is the value specified by 2nd cell above. If FlexRM
+		does not get required number of completion messages in time
+		specified by this cell then it will inject one MSI interrupt
+		to CPU provided atleast one completion message is available.
+
+Optional properties:
+--------------------
+- dma-coherent:	Present if DMA operations made by the FlexRM engine (such
+		as DMA descriptor access, access to buffers pointed by DMA
+		descriptors and read/write pointer updates to DDR) are
+		cache coherent with the CPU.
+
+Example:
+--------
+crypto_mbox: mbox@67000000 {
+	compatible = "brcm,iproc-flexrm-mbox";
+	reg = <0x67000000 0x200000>;
+	msi-parent = <&gic_its 0x7f00>;
+	#mbox-cells = <3>;
+};
+
+crypto@672c0000 {
+	compatible = "brcm,spu2-v2-crypto";
+	reg = <0x672c0000 0x1000>;
+	mboxes = <&crypto_mbox 0 0x1 0xffff>,
+		 <&crypto_mbox 1 0x1 0xffff>,
+		 <&crypto_mbox 16 0x1 0xffff>,
+		 <&crypto_mbox 17 0x1 0xffff>,
+		 <&crypto_mbox 30 0x1 0xffff>,
+		 <&crypto_mbox 31 0x1 0xffff>;
+};
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v7 1/3] mailbox: Add driver for Broadcom FlexRM ring manager
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree, linux-kernel, linux-arm-kernel,
	bcm-kernel-feedback-list, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel@broadcom.com>

Some of the Broadcom iProc SoCs have FlexRM ring manager
which provides a ring-based programming interface to various
offload engines (e.g. RAID, Crypto, etc).

This patch adds a common mailbox driver for Broadcom FlexRM
ring manager which can be shared by various offload engine
drivers (implemented as mailbox clients).

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Pramod KUMAR <pramod.kumar@broadcom.com>
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/mailbox/Kconfig              |   11 +
 drivers/mailbox/Makefile             |    2 +
 drivers/mailbox/bcm-flexrm-mailbox.c | 1595 ++++++++++++++++++++++++++++++++++
 include/linux/mailbox/brcm-message.h |   14 +-
 4 files changed, 1618 insertions(+), 4 deletions(-)
 create mode 100644 drivers/mailbox/bcm-flexrm-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ceff415..305018c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -152,4 +152,15 @@ config BCM_PDC_MBOX
 	  Mailbox implementation for the Broadcom PDC ring manager,
 	  which provides access to various offload engines on Broadcom
 	  SoCs. Say Y here if you want to use the Broadcom PDC.
+
+config BCM_FLEXRM_MBOX
+	tristate "Broadcom FlexRM Mailbox"
+	depends on ARM64 || COMPILE_TEST
+	depends on HAS_DMA
+	select GENERIC_MSI_IRQ_DOMAIN
+	default ARCH_BCM_IPROC
+	help
+	  Mailbox implementation of the Broadcom FlexRM ring manager,
+	  which provides access to various offload engines on Broadcom
+	  SoCs. Say Y here if you want to use the Broadcom FlexRM.
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 7dde4f6..e2bcb03 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -30,4 +30,6 @@ obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
 
 obj-$(CONFIG_BCM_PDC_MBOX)	+= bcm-pdc-mailbox.o
 
+obj-$(CONFIG_BCM_FLEXRM_MBOX)	+= bcm-flexrm-mailbox.o
+
 obj-$(CONFIG_TEGRA_HSP_MBOX)	+= tegra-hsp.o
diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
new file mode 100644
index 0000000..da67882
--- /dev/null
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -0,0 +1,1595 @@
+/* Broadcom FlexRM Mailbox Driver
+ *
+ * Copyright (C) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Each Broadcom FlexSparx4 offload engine is implemented as an
+ * extension to Broadcom FlexRM ring manager. The FlexRM ring
+ * manager provides a set of rings which can be used to submit
+ * work to a FlexSparx4 offload engine.
+ *
+ * This driver creates a mailbox controller using a set of FlexRM
+ * rings where each mailbox channel represents a separate FlexRM ring.
+ */
+
+#include <asm/barrier.h>
+#include <asm/byteorder.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/err.h>
+#include <linux/idr.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/brcm-message.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* ====== FlexRM register defines ===== */
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE					0x10000
+#define RING_DESC_SIZE					8
+#define RING_DESC_INDEX(offset)				\
+			((offset) / RING_DESC_SIZE)
+#define RING_DESC_OFFSET(index)				\
+			((index) * RING_DESC_SIZE)
+#define RING_MAX_REQ_COUNT				1024
+#define RING_BD_ALIGN_ORDER				12
+#define RING_BD_ALIGN_CHECK(addr)			\
+			(!((addr) & ((0x1 << RING_BD_ALIGN_ORDER) - 1)))
+#define RING_BD_TOGGLE_INVALID(offset)			\
+			(((offset) >> RING_BD_ALIGN_ORDER) & 0x1)
+#define RING_BD_TOGGLE_VALID(offset)			\
+			(!RING_BD_TOGGLE_INVALID(offset))
+#define RING_BD_DESC_PER_REQ				32
+#define RING_BD_DESC_COUNT				\
+			(RING_MAX_REQ_COUNT * RING_BD_DESC_PER_REQ)
+#define RING_BD_SIZE					\
+			(RING_BD_DESC_COUNT * RING_DESC_SIZE)
+#define RING_CMPL_ALIGN_ORDER				13
+#define RING_CMPL_DESC_COUNT				RING_MAX_REQ_COUNT
+#define RING_CMPL_SIZE					\
+			(RING_CMPL_DESC_COUNT * RING_DESC_SIZE)
+#define RING_VER_MAGIC					0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER					0x000
+#define RING_BD_START_ADDR				0x004
+#define RING_BD_READ_PTR				0x008
+#define RING_BD_WRITE_PTR				0x00c
+#define RING_BD_READ_PTR_DDR_LS				0x010
+#define RING_BD_READ_PTR_DDR_MS				0x014
+#define RING_CMPL_START_ADDR				0x018
+#define RING_CMPL_WRITE_PTR				0x01c
+#define RING_NUM_REQ_RECV_LS				0x020
+#define RING_NUM_REQ_RECV_MS				0x024
+#define RING_NUM_REQ_TRANS_LS				0x028
+#define RING_NUM_REQ_TRANS_MS				0x02c
+#define RING_NUM_REQ_OUTSTAND				0x030
+#define RING_CONTROL					0x034
+#define RING_FLUSH_DONE					0x038
+#define RING_MSI_ADDR_LS				0x03c
+#define RING_MSI_ADDR_MS				0x040
+#define RING_MSI_CONTROL				0x048
+#define RING_BD_READ_PTR_DDR_CONTROL			0x04c
+#define RING_MSI_DATA_VALUE				0x064
+
+/* Register RING_BD_START_ADDR fields */
+#define BD_LAST_UPDATE_HW_SHIFT				28
+#define BD_LAST_UPDATE_HW_MASK				0x1
+#define BD_START_ADDR_VALUE(pa)				\
+	((u32)((((dma_addr_t)(pa)) >> RING_BD_ALIGN_ORDER) & 0x0fffffff))
+#define BD_START_ADDR_DECODE(val)			\
+	((dma_addr_t)((val) & 0x0fffffff) << RING_BD_ALIGN_ORDER)
+
+/* Register RING_CMPL_START_ADDR fields */
+#define CMPL_START_ADDR_VALUE(pa)			\
+	((u32)((((u64)(pa)) >> RING_CMPL_ALIGN_ORDER) & 0x03ffffff))
+
+/* Register RING_CONTROL fields */
+#define CONTROL_MASK_DISABLE_CONTROL			12
+#define CONTROL_FLUSH_SHIFT				5
+#define CONTROL_ACTIVE_SHIFT				4
+#define CONTROL_RATE_ADAPT_MASK				0xf
+#define CONTROL_RATE_DYNAMIC				0x0
+#define CONTROL_RATE_FAST				0x8
+#define CONTROL_RATE_MEDIUM				0x9
+#define CONTROL_RATE_SLOW				0xa
+#define CONTROL_RATE_IDLE				0xb
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK					0x1
+
+/* Register RING_MSI_CONTROL fields */
+#define MSI_TIMER_VAL_SHIFT				16
+#define MSI_TIMER_VAL_MASK				0xffff
+#define MSI_ENABLE_SHIFT				15
+#define MSI_ENABLE_MASK					0x1
+#define MSI_COUNT_SHIFT					0
+#define MSI_COUNT_MASK					0x3ff
+
+/* Register RING_BD_READ_PTR_DDR_CONTROL fields */
+#define BD_READ_PTR_DDR_TIMER_VAL_SHIFT			16
+#define BD_READ_PTR_DDR_TIMER_VAL_MASK			0xffff
+#define BD_READ_PTR_DDR_ENABLE_SHIFT			15
+#define BD_READ_PTR_DDR_ENABLE_MASK			0x1
+
+/* ====== FlexRM ring descriptor defines ===== */
+
+/* Completion descriptor format */
+#define CMPL_OPAQUE_SHIFT			0
+#define CMPL_OPAQUE_MASK			0xffff
+#define CMPL_ENGINE_STATUS_SHIFT		16
+#define CMPL_ENGINE_STATUS_MASK			0xffff
+#define CMPL_DME_STATUS_SHIFT			32
+#define CMPL_DME_STATUS_MASK			0xffff
+#define CMPL_RM_STATUS_SHIFT			48
+#define CMPL_RM_STATUS_MASK			0xffff
+
+/* Completion DME status code */
+#define DME_STATUS_MEM_COR_ERR			BIT(0)
+#define DME_STATUS_MEM_UCOR_ERR			BIT(1)
+#define DME_STATUS_FIFO_UNDERFLOW		BIT(2)
+#define DME_STATUS_FIFO_OVERFLOW		BIT(3)
+#define DME_STATUS_RRESP_ERR			BIT(4)
+#define DME_STATUS_BRESP_ERR			BIT(5)
+#define DME_STATUS_ERROR_MASK			(DME_STATUS_MEM_COR_ERR | \
+						 DME_STATUS_MEM_UCOR_ERR | \
+						 DME_STATUS_FIFO_UNDERFLOW | \
+						 DME_STATUS_FIFO_OVERFLOW | \
+						 DME_STATUS_RRESP_ERR | \
+						 DME_STATUS_BRESP_ERR)
+
+/* Completion RM status code */
+#define RM_STATUS_CODE_SHIFT			0
+#define RM_STATUS_CODE_MASK			0x3ff
+#define RM_STATUS_CODE_GOOD			0x0
+#define RM_STATUS_CODE_AE_TIMEOUT		0x3ff
+
+/* General descriptor format */
+#define DESC_TYPE_SHIFT				60
+#define DESC_TYPE_MASK				0xf
+#define DESC_PAYLOAD_SHIFT			0
+#define DESC_PAYLOAD_MASK			0x0fffffffffffffff
+
+/* Null descriptor format  */
+#define NULL_TYPE				0
+#define NULL_TOGGLE_SHIFT			58
+#define NULL_TOGGLE_MASK			0x1
+
+/* Header descriptor format */
+#define HEADER_TYPE				1
+#define HEADER_TOGGLE_SHIFT			58
+#define HEADER_TOGGLE_MASK			0x1
+#define HEADER_ENDPKT_SHIFT			57
+#define HEADER_ENDPKT_MASK			0x1
+#define HEADER_STARTPKT_SHIFT			56
+#define HEADER_STARTPKT_MASK			0x1
+#define HEADER_BDCOUNT_SHIFT			36
+#define HEADER_BDCOUNT_MASK			0x1f
+#define HEADER_BDCOUNT_MAX			HEADER_BDCOUNT_MASK
+#define HEADER_FLAGS_SHIFT			16
+#define HEADER_FLAGS_MASK			0xffff
+#define HEADER_OPAQUE_SHIFT			0
+#define HEADER_OPAQUE_MASK			0xffff
+
+/* Source (SRC) descriptor format */
+#define SRC_TYPE				2
+#define SRC_LENGTH_SHIFT			44
+#define SRC_LENGTH_MASK				0xffff
+#define SRC_ADDR_SHIFT				0
+#define SRC_ADDR_MASK				0x00000fffffffffff
+
+/* Destination (DST) descriptor format */
+#define DST_TYPE				3
+#define DST_LENGTH_SHIFT			44
+#define DST_LENGTH_MASK				0xffff
+#define DST_ADDR_SHIFT				0
+#define DST_ADDR_MASK				0x00000fffffffffff
+
+/* Immediate (IMM) descriptor format */
+#define IMM_TYPE				4
+#define IMM_DATA_SHIFT				0
+#define IMM_DATA_MASK				0x0fffffffffffffff
+
+/* Next pointer (NPTR) descriptor format */
+#define NPTR_TYPE				5
+#define NPTR_TOGGLE_SHIFT			58
+#define NPTR_TOGGLE_MASK			0x1
+#define NPTR_ADDR_SHIFT				0
+#define NPTR_ADDR_MASK				0x00000fffffffffff
+
+/* Mega source (MSRC) descriptor format */
+#define MSRC_TYPE				6
+#define MSRC_LENGTH_SHIFT			44
+#define MSRC_LENGTH_MASK			0xffff
+#define MSRC_ADDR_SHIFT				0
+#define MSRC_ADDR_MASK				0x00000fffffffffff
+
+/* Mega destination (MDST) descriptor format */
+#define MDST_TYPE				7
+#define MDST_LENGTH_SHIFT			44
+#define MDST_LENGTH_MASK			0xffff
+#define MDST_ADDR_SHIFT				0
+#define MDST_ADDR_MASK				0x00000fffffffffff
+
+/* Source with tlast (SRCT) descriptor format */
+#define SRCT_TYPE				8
+#define SRCT_LENGTH_SHIFT			44
+#define SRCT_LENGTH_MASK			0xffff
+#define SRCT_ADDR_SHIFT				0
+#define SRCT_ADDR_MASK				0x00000fffffffffff
+
+/* Destination with tlast (DSTT) descriptor format */
+#define DSTT_TYPE				9
+#define DSTT_LENGTH_SHIFT			44
+#define DSTT_LENGTH_MASK			0xffff
+#define DSTT_ADDR_SHIFT				0
+#define DSTT_ADDR_MASK				0x00000fffffffffff
+
+/* Immediate with tlast (IMMT) descriptor format */
+#define IMMT_TYPE				10
+#define IMMT_DATA_SHIFT				0
+#define IMMT_DATA_MASK				0x0fffffffffffffff
+
+/* Descriptor helper macros */
+#define DESC_DEC(_d, _s, _m)			(((_d) >> (_s)) & (_m))
+#define DESC_ENC(_d, _v, _s, _m)		\
+			do { \
+				(_d) &= ~((u64)(_m) << (_s)); \
+				(_d) |= (((u64)(_v) & (_m)) << (_s)); \
+			} while (0)
+
+/* ====== FlexRM data structures ===== */
+
+struct flexrm_ring {
+	/* Unprotected members */
+	int num;
+	struct flexrm_mbox *mbox;
+	void __iomem *regs;
+	bool irq_requested;
+	unsigned int irq;
+	unsigned int msi_timer_val;
+	unsigned int msi_count_threshold;
+	struct ida requests_ida;
+	struct brcm_message *requests[RING_MAX_REQ_COUNT];
+	void *bd_base;
+	dma_addr_t bd_dma_base;
+	u32 bd_write_offset;
+	void *cmpl_base;
+	dma_addr_t cmpl_dma_base;
+	/* Protected members */
+	spinlock_t lock;
+	struct brcm_message *last_pending_msg;
+	u32 cmpl_read_offset;
+};
+
+struct flexrm_mbox {
+	struct device *dev;
+	void __iomem *regs;
+	u32 num_rings;
+	struct flexrm_ring *rings;
+	struct dma_pool *bd_pool;
+	struct dma_pool *cmpl_pool;
+	struct mbox_controller controller;
+};
+
+/* ====== FlexRM ring descriptor helper routines ===== */
+
+static u64 flexrm_read_desc(void *desc_ptr)
+{
+	return le64_to_cpu(*((u64 *)desc_ptr));
+}
+
+static void flexrm_write_desc(void *desc_ptr, u64 desc)
+{
+	*((u64 *)desc_ptr) = cpu_to_le64(desc);
+}
+
+static u32 flexrm_cmpl_desc_to_reqid(u64 cmpl_desc)
+{
+	return (u32)(cmpl_desc & CMPL_OPAQUE_MASK);
+}
+
+static int flexrm_cmpl_desc_to_error(u64 cmpl_desc)
+{
+	u32 status;
+
+	status = DESC_DEC(cmpl_desc, CMPL_DME_STATUS_SHIFT,
+			  CMPL_DME_STATUS_MASK);
+	if (status & DME_STATUS_ERROR_MASK)
+		return -EIO;
+
+	status = DESC_DEC(cmpl_desc, CMPL_RM_STATUS_SHIFT,
+			  CMPL_RM_STATUS_MASK);
+	status &= RM_STATUS_CODE_MASK;
+	if (status == RM_STATUS_CODE_AE_TIMEOUT)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static bool flexrm_is_next_table_desc(void *desc_ptr)
+{
+	u64 desc = flexrm_read_desc(desc_ptr);
+	u32 type = DESC_DEC(desc, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+
+	return (type == NPTR_TYPE) ? true : false;
+}
+
+static u64 flexrm_next_table_desc(u32 toggle, dma_addr_t next_addr)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, NPTR_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, NPTR_TOGGLE_SHIFT, NPTR_TOGGLE_MASK);
+	DESC_ENC(desc, next_addr, NPTR_ADDR_SHIFT, NPTR_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_null_desc(u32 toggle)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, NULL_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, NULL_TOGGLE_SHIFT, NULL_TOGGLE_MASK);
+
+	return desc;
+}
+
+static u32 flexrm_estimate_header_desc_count(u32 nhcnt)
+{
+	u32 hcnt = nhcnt / HEADER_BDCOUNT_MAX;
+
+	if (!(nhcnt % HEADER_BDCOUNT_MAX))
+		hcnt += 1;
+
+	return hcnt;
+}
+
+static void flexrm_flip_header_toogle(void *desc_ptr)
+{
+	u64 desc = flexrm_read_desc(desc_ptr);
+
+	if (desc & ((u64)0x1 << HEADER_TOGGLE_SHIFT))
+		desc &= ~((u64)0x1 << HEADER_TOGGLE_SHIFT);
+	else
+		desc |= ((u64)0x1 << HEADER_TOGGLE_SHIFT);
+
+	flexrm_write_desc(desc_ptr, desc);
+}
+
+static u64 flexrm_header_desc(u32 toggle, u32 startpkt, u32 endpkt,
+			       u32 bdcount, u32 flags, u32 opaque)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, HEADER_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, HEADER_TOGGLE_SHIFT, HEADER_TOGGLE_MASK);
+	DESC_ENC(desc, startpkt, HEADER_STARTPKT_SHIFT, HEADER_STARTPKT_MASK);
+	DESC_ENC(desc, endpkt, HEADER_ENDPKT_SHIFT, HEADER_ENDPKT_MASK);
+	DESC_ENC(desc, bdcount, HEADER_BDCOUNT_SHIFT, HEADER_BDCOUNT_MASK);
+	DESC_ENC(desc, flags, HEADER_FLAGS_SHIFT, HEADER_FLAGS_MASK);
+	DESC_ENC(desc, opaque, HEADER_OPAQUE_SHIFT, HEADER_OPAQUE_MASK);
+
+	return desc;
+}
+
+static void flexrm_enqueue_desc(u32 nhpos, u32 nhcnt, u32 reqid,
+				 u64 desc, void **desc_ptr, u32 *toggle,
+				 void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 nhavail, _toggle, _startpkt, _endpkt, _bdcount;
+
+	/* Sanity check */
+	if (nhcnt <= nhpos)
+		return;
+
+	/*
+	 * Each request or packet start with a HEADER descriptor followed
+	 * by one or more non-HEADER descriptors (SRC, SRCT, MSRC, DST,
+	 * DSTT, MDST, IMM, and IMMT). The number of non-HEADER descriptors
+	 * following a HEADER descriptor is represented by BDCOUNT field
+	 * of HEADER descriptor. The max value of BDCOUNT field is 31 which
+	 * means we can only have 31 non-HEADER descriptors following one
+	 * HEADER descriptor.
+	 *
+	 * In general use, number of non-HEADER descriptors can easily go
+	 * beyond 31. To tackle this situation, we have packet (or request)
+	 * extenstion bits (STARTPKT and ENDPKT) in the HEADER descriptor.
+	 *
+	 * To use packet extension, the first HEADER descriptor of request
+	 * (or packet) will have STARTPKT=1 and ENDPKT=0. The intermediate
+	 * HEADER descriptors will have STARTPKT=0 and ENDPKT=0. The last
+	 * HEADER descriptor will have STARTPKT=0 and ENDPKT=1. Also, the
+	 * TOGGLE bit of the first HEADER will be set to invalid state to
+	 * ensure that FlexRM does not start fetching descriptors till all
+	 * descriptors are enqueued. The user of this function will flip
+	 * the TOGGLE bit of first HEADER after all descriptors are
+	 * enqueued.
+	 */
+
+	if ((nhpos % HEADER_BDCOUNT_MAX == 0) && (nhcnt - nhpos)) {
+		/* Prepare the header descriptor */
+		nhavail = (nhcnt - nhpos);
+		_toggle = (nhpos == 0) ? !(*toggle) : (*toggle);
+		_startpkt = (nhpos == 0) ? 0x1 : 0x0;
+		_endpkt = (nhavail <= HEADER_BDCOUNT_MAX) ? 0x1 : 0x0;
+		_bdcount = (nhavail <= HEADER_BDCOUNT_MAX) ?
+				nhavail : HEADER_BDCOUNT_MAX;
+		if (nhavail <= HEADER_BDCOUNT_MAX)
+			_bdcount = nhavail;
+		else
+			_bdcount = HEADER_BDCOUNT_MAX;
+		d = flexrm_header_desc(_toggle, _startpkt, _endpkt,
+					_bdcount, 0x0, reqid);
+
+		/* Write header descriptor */
+		flexrm_write_desc(*desc_ptr, d);
+
+		/* Point to next descriptor */
+		*desc_ptr += sizeof(desc);
+		if (*desc_ptr == end_desc)
+			*desc_ptr = start_desc;
+
+		/* Skip next pointer descriptors */
+		while (flexrm_is_next_table_desc(*desc_ptr)) {
+			*toggle = (*toggle) ? 0 : 1;
+			*desc_ptr += sizeof(desc);
+			if (*desc_ptr == end_desc)
+				*desc_ptr = start_desc;
+		}
+	}
+
+	/* Write desired descriptor */
+	flexrm_write_desc(*desc_ptr, desc);
+
+	/* Point to next descriptor */
+	*desc_ptr += sizeof(desc);
+	if (*desc_ptr == end_desc)
+		*desc_ptr = start_desc;
+
+	/* Skip next pointer descriptors */
+	while (flexrm_is_next_table_desc(*desc_ptr)) {
+		*toggle = (*toggle) ? 0 : 1;
+		*desc_ptr += sizeof(desc);
+		if (*desc_ptr == end_desc)
+			*desc_ptr = start_desc;
+	}
+}
+
+static u64 flexrm_src_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, SRC_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, SRC_LENGTH_SHIFT, SRC_LENGTH_MASK);
+	DESC_ENC(desc, addr, SRC_ADDR_SHIFT, SRC_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_msrc_desc(dma_addr_t addr, unsigned int length_div_16)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, MSRC_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length_div_16, MSRC_LENGTH_SHIFT, MSRC_LENGTH_MASK);
+	DESC_ENC(desc, addr, MSRC_ADDR_SHIFT, MSRC_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_dst_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, DST_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, DST_LENGTH_SHIFT, DST_LENGTH_MASK);
+	DESC_ENC(desc, addr, DST_ADDR_SHIFT, DST_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_mdst_desc(dma_addr_t addr, unsigned int length_div_16)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, MDST_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length_div_16, MDST_LENGTH_SHIFT, MDST_LENGTH_MASK);
+	DESC_ENC(desc, addr, MDST_ADDR_SHIFT, MDST_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_imm_desc(u64 data)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, IMM_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, data, IMM_DATA_SHIFT, IMM_DATA_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_srct_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, SRCT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, SRCT_LENGTH_SHIFT, SRCT_LENGTH_MASK);
+	DESC_ENC(desc, addr, SRCT_ADDR_SHIFT, SRCT_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_dstt_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, DSTT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, DSTT_LENGTH_SHIFT, DSTT_LENGTH_MASK);
+	DESC_ENC(desc, addr, DSTT_ADDR_SHIFT, DSTT_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_immt_desc(u64 data)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, IMMT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, data, IMMT_DATA_SHIFT, IMMT_DATA_MASK);
+
+	return desc;
+}
+
+static bool flexrm_spu_sanity_check(struct brcm_message *msg)
+{
+	struct scatterlist *sg;
+
+	if (!msg->spu.src || !msg->spu.dst)
+		return false;
+	for (sg = msg->spu.src; sg; sg = sg_next(sg)) {
+		if (sg->length & 0xf) {
+			if (sg->length > SRC_LENGTH_MASK)
+				return false;
+		} else {
+			if (sg->length > (MSRC_LENGTH_MASK * 16))
+				return false;
+		}
+	}
+	for (sg = msg->spu.dst; sg; sg = sg_next(sg)) {
+		if (sg->length & 0xf) {
+			if (sg->length > DST_LENGTH_MASK)
+				return false;
+		} else {
+			if (sg->length > (MDST_LENGTH_MASK * 16))
+				return false;
+		}
+	}
+
+	return true;
+}
+
+static u32 flexrm_spu_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	u32 cnt = 0;
+	unsigned int dst_target = 0;
+	struct scatterlist *src_sg = msg->spu.src, *dst_sg = msg->spu.dst;
+
+	while (src_sg || dst_sg) {
+		if (src_sg) {
+			cnt++;
+			dst_target = src_sg->length;
+			src_sg = sg_next(src_sg);
+		} else
+			dst_target = UINT_MAX;
+
+		while (dst_target && dst_sg) {
+			cnt++;
+			if (dst_sg->length < dst_target)
+				dst_target -= dst_sg->length;
+			else
+				dst_target = 0;
+			dst_sg = sg_next(dst_sg);
+		}
+	}
+
+	return cnt;
+}
+
+static int flexrm_spu_dma_map(struct device *dev, struct brcm_message *msg)
+{
+	int rc;
+
+	rc = dma_map_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+			DMA_TO_DEVICE);
+	if (rc < 0)
+		return rc;
+
+	rc = dma_map_sg(dev, msg->spu.dst, sg_nents(msg->spu.dst),
+			DMA_FROM_DEVICE);
+	if (rc < 0) {
+		dma_unmap_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+			     DMA_TO_DEVICE);
+		return rc;
+	}
+
+	return 0;
+}
+
+static void flexrm_spu_dma_unmap(struct device *dev, struct brcm_message *msg)
+{
+	dma_unmap_sg(dev, msg->spu.dst, sg_nents(msg->spu.dst),
+		     DMA_FROM_DEVICE);
+	dma_unmap_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+		     DMA_TO_DEVICE);
+}
+
+static void *flexrm_spu_write_descs(struct brcm_message *msg, u32 nhcnt,
+				     u32 reqid, void *desc_ptr, u32 toggle,
+				     void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 nhpos = 0;
+	void *orig_desc_ptr = desc_ptr;
+	unsigned int dst_target = 0;
+	struct scatterlist *src_sg = msg->spu.src, *dst_sg = msg->spu.dst;
+
+	while (src_sg || dst_sg) {
+		if (src_sg) {
+			if (sg_dma_len(src_sg) & 0xf)
+				d = flexrm_src_desc(sg_dma_address(src_sg),
+						     sg_dma_len(src_sg));
+			else
+				d = flexrm_msrc_desc(sg_dma_address(src_sg),
+						      sg_dma_len(src_sg)/16);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+			dst_target = sg_dma_len(src_sg);
+			src_sg = sg_next(src_sg);
+		} else
+			dst_target = UINT_MAX;
+
+		while (dst_target && dst_sg) {
+			if (sg_dma_len(dst_sg) & 0xf)
+				d = flexrm_dst_desc(sg_dma_address(dst_sg),
+						     sg_dma_len(dst_sg));
+			else
+				d = flexrm_mdst_desc(sg_dma_address(dst_sg),
+						      sg_dma_len(dst_sg)/16);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+			if (sg_dma_len(dst_sg) < dst_target)
+				dst_target -= sg_dma_len(dst_sg);
+			else
+				dst_target = 0;
+			dst_sg = sg_next(dst_sg);
+		}
+	}
+
+	/* Null descriptor with invalid toggle bit */
+	flexrm_write_desc(desc_ptr, flexrm_null_desc(!toggle));
+
+	/* Ensure that descriptors have been written to memory */
+	wmb();
+
+	/* Flip toggle bit in header */
+	flexrm_flip_header_toogle(orig_desc_ptr);
+
+	return desc_ptr;
+}
+
+static bool flexrm_sba_sanity_check(struct brcm_message *msg)
+{
+	u32 i;
+
+	if (!msg->sba.cmds || !msg->sba.cmds_count)
+		return false;
+
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		if (((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) ||
+		     (msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C)) &&
+		    (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) &&
+		    (msg->sba.cmds[i].data_len > SRCT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C) &&
+		    (msg->sba.cmds[i].data_len > SRCT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_RESP) &&
+		    (msg->sba.cmds[i].resp_len > DSTT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT) &&
+		    (msg->sba.cmds[i].data_len > DSTT_LENGTH_MASK))
+			return false;
+	}
+
+	return true;
+}
+
+static u32 flexrm_sba_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	u32 i, cnt;
+
+	cnt = 0;
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		cnt++;
+
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) ||
+		    (msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C))
+			cnt++;
+
+		if (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_RESP)
+			cnt++;
+
+		if (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT)
+			cnt++;
+	}
+
+	return cnt;
+}
+
+static void *flexrm_sba_write_descs(struct brcm_message *msg, u32 nhcnt,
+				     u32 reqid, void *desc_ptr, u32 toggle,
+				     void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 i, nhpos = 0;
+	struct brcm_sba_command *c;
+	void *orig_desc_ptr = desc_ptr;
+
+	/* Convert SBA commands into descriptors */
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		c = &msg->sba.cmds[i];
+
+		if ((c->flags & BRCM_SBA_CMD_HAS_RESP) &&
+		    (c->flags & BRCM_SBA_CMD_HAS_OUTPUT)) {
+			/* Destination response descriptor */
+			d = flexrm_dst_desc(c->resp, c->resp_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		} else if (c->flags & BRCM_SBA_CMD_HAS_RESP) {
+			/* Destination response with tlast descriptor */
+			d = flexrm_dstt_desc(c->resp, c->resp_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if (c->flags & BRCM_SBA_CMD_HAS_OUTPUT) {
+			/* Destination with tlast descriptor */
+			d = flexrm_dstt_desc(c->data, c->data_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if (c->flags & BRCM_SBA_CMD_TYPE_B) {
+			/* Command as immediate descriptor */
+			d = flexrm_imm_desc(c->cmd);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		} else {
+			/* Command as immediate descriptor with tlast */
+			d = flexrm_immt_desc(c->cmd);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if ((c->flags & BRCM_SBA_CMD_TYPE_B) ||
+		    (c->flags & BRCM_SBA_CMD_TYPE_C)) {
+			/* Source with tlast descriptor */
+			d = flexrm_srct_desc(c->data, c->data_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+	}
+
+	/* Null descriptor with invalid toggle bit */
+	flexrm_write_desc(desc_ptr, flexrm_null_desc(!toggle));
+
+	/* Ensure that descriptors have been written to memory */
+	wmb();
+
+	/* Flip toggle bit in header */
+	flexrm_flip_header_toogle(orig_desc_ptr);
+
+	return desc_ptr;
+}
+
+static bool flexrm_sanity_check(struct brcm_message *msg)
+{
+	if (!msg)
+		return false;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_sanity_check(msg);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_sanity_check(msg);
+	default:
+		return false;
+	};
+}
+
+static u32 flexrm_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	if (!msg)
+		return 0;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_estimate_nonheader_desc_count(msg);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_estimate_nonheader_desc_count(msg);
+	default:
+		return 0;
+	};
+}
+
+static int flexrm_dma_map(struct device *dev, struct brcm_message *msg)
+{
+	if (!dev || !msg)
+		return -EINVAL;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_dma_map(dev, msg);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void flexrm_dma_unmap(struct device *dev, struct brcm_message *msg)
+{
+	if (!dev || !msg)
+		return;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		flexrm_spu_dma_unmap(dev, msg);
+		break;
+	default:
+		break;
+	}
+}
+
+static void *flexrm_write_descs(struct brcm_message *msg, u32 nhcnt,
+				u32 reqid, void *desc_ptr, u32 toggle,
+				void *start_desc, void *end_desc)
+{
+	if (!msg || !desc_ptr || !start_desc || !end_desc)
+		return ERR_PTR(-ENOTSUPP);
+
+	if ((desc_ptr < start_desc) || (end_desc <= desc_ptr))
+		return ERR_PTR(-ERANGE);
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_write_descs(msg, nhcnt, reqid,
+					       desc_ptr, toggle,
+					       start_desc, end_desc);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_write_descs(msg, nhcnt, reqid,
+					       desc_ptr, toggle,
+					       start_desc, end_desc);
+	default:
+		return ERR_PTR(-ENOTSUPP);
+	};
+}
+
+/* ====== FlexRM driver helper routines ===== */
+
+static int flexrm_new_request(struct flexrm_ring *ring,
+				struct brcm_message *batch_msg,
+				struct brcm_message *msg)
+{
+	void *next;
+	unsigned long flags;
+	u32 val, count, nhcnt;
+	u32 read_offset, write_offset;
+	bool exit_cleanup = false;
+	int ret = 0, reqid;
+
+	/* Do sanity check on message */
+	if (!flexrm_sanity_check(msg))
+		return -EIO;
+	msg->error = 0;
+
+	/* If no requests possible then save data pointer and goto done. */
+	reqid = ida_simple_get(&ring->requests_ida, 0,
+				RING_MAX_REQ_COUNT, GFP_KERNEL);
+	if (reqid < 0) {
+		spin_lock_irqsave(&ring->lock, flags);
+		if (batch_msg)
+			ring->last_pending_msg = batch_msg;
+		else
+			ring->last_pending_msg = msg;
+		spin_unlock_irqrestore(&ring->lock, flags);
+		return 0;
+	}
+	ring->requests[reqid] = msg;
+
+	/* Do DMA mappings for the message */
+	ret = flexrm_dma_map(ring->mbox->dev, msg);
+	if (ret < 0) {
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+		return ret;
+	}
+
+	/* If last_pending_msg is already set then goto done with error */
+	spin_lock_irqsave(&ring->lock, flags);
+	if (ring->last_pending_msg)
+		ret = -ENOSPC;
+	spin_unlock_irqrestore(&ring->lock, flags);
+	if (ret < 0) {
+		dev_warn(ring->mbox->dev, "no space in ring %d\n", ring->num);
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Determine current HW BD read offset */
+	read_offset = readl_relaxed(ring->regs + RING_BD_READ_PTR);
+	val = readl_relaxed(ring->regs + RING_BD_START_ADDR);
+	read_offset *= RING_DESC_SIZE;
+	read_offset += (u32)(BD_START_ADDR_DECODE(val) - ring->bd_dma_base);
+
+	/*
+	 * Number required descriptors = number of non-header descriptors +
+	 *				 number of header descriptors +
+	 *				 1x null descriptor
+	 */
+	nhcnt = flexrm_estimate_nonheader_desc_count(msg);
+	count = flexrm_estimate_header_desc_count(nhcnt) + nhcnt + 1;
+
+	/* Check for available descriptor space. */
+	write_offset = ring->bd_write_offset;
+	while (count) {
+		if (!flexrm_is_next_table_desc(ring->bd_base + write_offset))
+			count--;
+		write_offset += RING_DESC_SIZE;
+		if (write_offset == RING_BD_SIZE)
+			write_offset = 0x0;
+		if (write_offset == read_offset)
+			break;
+	}
+	if (count) {
+		spin_lock_irqsave(&ring->lock, flags);
+		if (batch_msg)
+			ring->last_pending_msg = batch_msg;
+		else
+			ring->last_pending_msg = msg;
+		spin_unlock_irqrestore(&ring->lock, flags);
+		ret = 0;
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Write descriptors to ring */
+	next = flexrm_write_descs(msg, nhcnt, reqid,
+			ring->bd_base + ring->bd_write_offset,
+			RING_BD_TOGGLE_VALID(ring->bd_write_offset),
+			ring->bd_base, ring->bd_base + RING_BD_SIZE);
+	if (IS_ERR(next)) {
+		ret = PTR_ERR(next);
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Save ring BD write offset */
+	ring->bd_write_offset = (unsigned long)(next - ring->bd_base);
+
+exit:
+	/* Update error status in message */
+	msg->error = ret;
+
+	/* Cleanup if we failed */
+	if (exit_cleanup) {
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+	}
+
+	return ret;
+}
+
+static int flexrm_process_completions(struct flexrm_ring *ring)
+{
+	u64 desc;
+	int err, count = 0;
+	unsigned long flags;
+	struct brcm_message *msg = NULL;
+	u32 reqid, cmpl_read_offset, cmpl_write_offset;
+	struct mbox_chan *chan = &ring->mbox->controller.chans[ring->num];
+
+	spin_lock_irqsave(&ring->lock, flags);
+
+	/* Check last_pending_msg */
+	if (ring->last_pending_msg) {
+		msg = ring->last_pending_msg;
+		ring->last_pending_msg = NULL;
+	}
+
+	/*
+	 * Get current completion read and write offset
+	 *
+	 * Note: We should read completion write pointer atleast once
+	 * after we get a MSI interrupt because HW maintains internal
+	 * MSI status which will allow next MSI interrupt only after
+	 * completion write pointer is read.
+	 */
+	cmpl_write_offset = readl_relaxed(ring->regs + RING_CMPL_WRITE_PTR);
+	cmpl_write_offset *= RING_DESC_SIZE;
+	cmpl_read_offset = ring->cmpl_read_offset;
+	ring->cmpl_read_offset = cmpl_write_offset;
+
+	spin_unlock_irqrestore(&ring->lock, flags);
+
+	/* If last_pending_msg was set then queue it back */
+	if (msg)
+		mbox_send_message(chan, msg);
+
+	/* For each completed request notify mailbox clients */
+	reqid = 0;
+	while (cmpl_read_offset != cmpl_write_offset) {
+		/* Dequeue next completion descriptor */
+		desc = *((u64 *)(ring->cmpl_base + cmpl_read_offset));
+
+		/* Next read offset */
+		cmpl_read_offset += RING_DESC_SIZE;
+		if (cmpl_read_offset == RING_CMPL_SIZE)
+			cmpl_read_offset = 0;
+
+		/* Decode error from completion descriptor */
+		err = flexrm_cmpl_desc_to_error(desc);
+		if (err < 0) {
+			dev_warn(ring->mbox->dev,
+				 "got completion desc=0x%lx with error %d",
+				 (unsigned long)desc, err);
+		}
+
+		/* Determine request id from completion descriptor */
+		reqid = flexrm_cmpl_desc_to_reqid(desc);
+
+		/* Determine message pointer based on reqid */
+		msg = ring->requests[reqid];
+		if (!msg) {
+			dev_warn(ring->mbox->dev,
+				 "null msg pointer for completion desc=0x%lx",
+				 (unsigned long)desc);
+			continue;
+		}
+
+		/* Release reqid for recycling */
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+
+		/* Unmap DMA mappings */
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+
+		/* Give-back message to mailbox client */
+		msg->error = err;
+		mbox_chan_received_data(chan, msg);
+
+		/* Increment number of completions processed */
+		count++;
+	}
+
+	return count;
+}
+
+/* ====== FlexRM interrupt handler ===== */
+
+static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
+{
+	/* We only have MSI for completions so just wakeup IRQ thread */
+	/* Ring related errors will be informed via completion descriptors */
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
+{
+	flexrm_process_completions(dev_id);
+
+	return IRQ_HANDLED;
+}
+
+/* ====== FlexRM mailbox callbacks ===== */
+
+static int flexrm_send_data(struct mbox_chan *chan, void *data)
+{
+	int i, rc;
+	struct flexrm_ring *ring = chan->con_priv;
+	struct brcm_message *msg = data;
+
+	if (msg->type == BRCM_MESSAGE_BATCH) {
+		for (i = msg->batch.msgs_queued;
+		     i < msg->batch.msgs_count; i++) {
+			rc = flexrm_new_request(ring, msg,
+						 &msg->batch.msgs[i]);
+			if (rc) {
+				msg->error = rc;
+				return rc;
+			}
+			msg->batch.msgs_queued++;
+		}
+		return 0;
+	}
+
+	return flexrm_new_request(ring, NULL, data);
+}
+
+static bool flexrm_peek_data(struct mbox_chan *chan)
+{
+	int cnt = flexrm_process_completions(chan->con_priv);
+
+	return (cnt > 0) ? true : false;
+}
+
+static int flexrm_startup(struct mbox_chan *chan)
+{
+	u64 d;
+	u32 val, off;
+	int ret = 0;
+	dma_addr_t next_addr;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	/* Allocate BD memory */
+	ring->bd_base = dma_pool_alloc(ring->mbox->bd_pool,
+				       GFP_KERNEL, &ring->bd_dma_base);
+	if (!ring->bd_base) {
+		dev_err(ring->mbox->dev, "can't allocate BD memory\n");
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	/* Configure next table pointer entries in BD memory */
+	for (off = 0; off < RING_BD_SIZE; off += RING_DESC_SIZE) {
+		next_addr = off + RING_DESC_SIZE;
+		if (next_addr == RING_BD_SIZE)
+			next_addr = 0;
+		next_addr += ring->bd_dma_base;
+		if (RING_BD_ALIGN_CHECK(next_addr))
+			d = flexrm_next_table_desc(RING_BD_TOGGLE_VALID(off),
+						    next_addr);
+		else
+			d = flexrm_null_desc(RING_BD_TOGGLE_INVALID(off));
+		flexrm_write_desc(ring->bd_base + off, d);
+	}
+
+	/* Allocate completion memory */
+	ring->cmpl_base = dma_pool_alloc(ring->mbox->cmpl_pool,
+					 GFP_KERNEL, &ring->cmpl_dma_base);
+	if (!ring->cmpl_base) {
+		dev_err(ring->mbox->dev, "can't allocate completion memory\n");
+		ret = -ENOMEM;
+		goto fail_free_bd_memory;
+	}
+	memset(ring->cmpl_base, 0, RING_CMPL_SIZE);
+
+	/* Request IRQ */
+	if (ring->irq == UINT_MAX) {
+		dev_err(ring->mbox->dev, "ring IRQ not available\n");
+		ret = -ENODEV;
+		goto fail_free_cmpl_memory;
+	}
+	ret = request_threaded_irq(ring->irq,
+				   flexrm_irq_event,
+				   flexrm_irq_thread,
+				   0, dev_name(ring->mbox->dev), ring);
+	if (ret) {
+		dev_err(ring->mbox->dev, "failed to request ring IRQ\n");
+		goto fail_free_cmpl_memory;
+	}
+	ring->irq_requested = true;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring->regs + RING_CONTROL);
+
+	/* Program BD start address */
+	val = BD_START_ADDR_VALUE(ring->bd_dma_base);
+	writel_relaxed(val, ring->regs + RING_BD_START_ADDR);
+
+	/* BD write pointer will be same as HW write pointer */
+	ring->bd_write_offset =
+			readl_relaxed(ring->regs + RING_BD_WRITE_PTR);
+	ring->bd_write_offset *= RING_DESC_SIZE;
+
+	/* Program completion start address */
+	val = CMPL_START_ADDR_VALUE(ring->cmpl_dma_base);
+	writel_relaxed(val, ring->regs + RING_CMPL_START_ADDR);
+
+	/* Ensure last pending message is cleared */
+	ring->last_pending_msg = NULL;
+
+	/* Completion read pointer will be same as HW write pointer */
+	ring->cmpl_read_offset =
+			readl_relaxed(ring->regs + RING_CMPL_WRITE_PTR);
+	ring->cmpl_read_offset *= RING_DESC_SIZE;
+
+	/* Read ring Tx, Rx, and Outstanding counts to clear */
+	readl_relaxed(ring->regs + RING_NUM_REQ_RECV_LS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_RECV_MS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_TRANS_LS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_TRANS_MS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_OUTSTAND);
+
+	/* Configure RING_MSI_CONTROL */
+	val = 0;
+	val |= (ring->msi_timer_val << MSI_TIMER_VAL_SHIFT);
+	val |= BIT(MSI_ENABLE_SHIFT);
+	val |= (ring->msi_count_threshold & MSI_COUNT_MASK) << MSI_COUNT_SHIFT;
+	writel_relaxed(val, ring->regs + RING_MSI_CONTROL);
+
+	/* Enable/activate ring */
+	val = BIT(CONTROL_ACTIVE_SHIFT);
+	writel_relaxed(val, ring->regs + RING_CONTROL);
+
+	return 0;
+
+fail_free_cmpl_memory:
+	dma_pool_free(ring->mbox->cmpl_pool,
+		      ring->cmpl_base, ring->cmpl_dma_base);
+	ring->cmpl_base = NULL;
+fail_free_bd_memory:
+	dma_pool_free(ring->mbox->bd_pool,
+		      ring->bd_base, ring->bd_dma_base);
+	ring->bd_base = NULL;
+fail:
+	return ret;
+}
+
+static void flexrm_shutdown(struct mbox_chan *chan)
+{
+	u32 reqid;
+	unsigned int timeout;
+	struct brcm_message *msg;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring->regs + RING_CONTROL);
+
+	/* Flush ring with timeout of 1s */
+	timeout = 1000;
+	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT),
+			ring->regs + RING_CONTROL);
+	do {
+		if (readl_relaxed(ring->regs + RING_FLUSH_DONE) &
+		    FLUSH_DONE_MASK)
+			break;
+		mdelay(1);
+	} while (timeout--);
+
+	/* Abort all in-flight requests */
+	for (reqid = 0; reqid < RING_MAX_REQ_COUNT; reqid++) {
+		msg = ring->requests[reqid];
+		if (!msg)
+			continue;
+
+		/* Release reqid for recycling */
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+
+		/* Unmap DMA mappings */
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+
+		/* Give-back message to mailbox client */
+		msg->error = -EIO;
+		mbox_chan_received_data(chan, msg);
+	}
+
+	/* Release IRQ */
+	if (ring->irq_requested) {
+		free_irq(ring->irq, ring);
+		ring->irq_requested = false;
+	}
+
+	/* Free-up completion descriptor ring */
+	if (ring->cmpl_base) {
+		dma_pool_free(ring->mbox->cmpl_pool,
+			      ring->cmpl_base, ring->cmpl_dma_base);
+		ring->cmpl_base = NULL;
+	}
+
+	/* Free-up BD descriptor ring */
+	if (ring->bd_base) {
+		dma_pool_free(ring->mbox->bd_pool,
+			      ring->bd_base, ring->bd_dma_base);
+		ring->bd_base = NULL;
+	}
+}
+
+static bool flexrm_last_tx_done(struct mbox_chan *chan)
+{
+	bool ret;
+	unsigned long flags;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	spin_lock_irqsave(&ring->lock, flags);
+	ret = (ring->last_pending_msg) ? false : true;
+	spin_unlock_irqrestore(&ring->lock, flags);
+
+	return ret;
+}
+
+static const struct mbox_chan_ops flexrm_mbox_chan_ops = {
+	.send_data	= flexrm_send_data,
+	.startup	= flexrm_startup,
+	.shutdown	= flexrm_shutdown,
+	.last_tx_done	= flexrm_last_tx_done,
+	.peek_data	= flexrm_peek_data,
+};
+
+static struct mbox_chan *flexrm_mbox_of_xlate(struct mbox_controller *cntlr,
+					const struct of_phandle_args *pa)
+{
+	struct mbox_chan *chan;
+	struct flexrm_ring *ring;
+
+	if (pa->args_count < 3)
+		return ERR_PTR(-EINVAL);
+
+	if (pa->args[0] >= cntlr->num_chans)
+		return ERR_PTR(-ENOENT);
+
+	if (pa->args[1] > MSI_COUNT_MASK)
+		return ERR_PTR(-EINVAL);
+
+	if (pa->args[2] > MSI_TIMER_VAL_MASK)
+		return ERR_PTR(-EINVAL);
+
+	chan = &cntlr->chans[pa->args[0]];
+	ring = chan->con_priv;
+	ring->msi_count_threshold = pa->args[1];
+	ring->msi_timer_val = pa->args[2];
+
+	return chan;
+}
+
+/* ====== FlexRM platform driver ===== */
+
+static void flexrm_mbox_msi_write(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct device *dev = msi_desc_to_dev(desc);
+	struct flexrm_mbox *mbox = dev_get_drvdata(dev);
+	struct flexrm_ring *ring = &mbox->rings[desc->platform.msi_index];
+
+	/* Configure per-Ring MSI registers */
+	writel_relaxed(msg->address_lo, ring->regs + RING_MSI_ADDR_LS);
+	writel_relaxed(msg->address_hi, ring->regs + RING_MSI_ADDR_MS);
+	writel_relaxed(msg->data, ring->regs + RING_MSI_DATA_VALUE);
+}
+
+static int flexrm_mbox_probe(struct platform_device *pdev)
+{
+	int index, ret = 0;
+	void __iomem *regs;
+	void __iomem *regs_end;
+	struct msi_desc *desc;
+	struct resource *iomem;
+	struct flexrm_ring *ring;
+	struct flexrm_mbox *mbox;
+	struct device *dev = &pdev->dev;
+
+	/* Allocate driver mailbox struct */
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	mbox->dev = dev;
+	platform_set_drvdata(pdev, mbox);
+
+	/* Get resource for registers */
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!iomem || (resource_size(iomem) < RING_REGS_SIZE)) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	/* Map registers of all rings */
+	mbox->regs = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(mbox->regs)) {
+		ret = PTR_ERR(mbox->regs);
+		dev_err(&pdev->dev, "Failed to remap mailbox regs: %d\n", ret);
+		goto fail;
+	}
+	regs_end = mbox->regs + resource_size(iomem);
+
+	/* Scan and count available rings */
+	mbox->num_rings = 0;
+	for (regs = mbox->regs; regs < regs_end; regs += RING_REGS_SIZE) {
+		if (readl_relaxed(regs + RING_VER) == RING_VER_MAGIC)
+			mbox->num_rings++;
+	}
+	if (!mbox->num_rings) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	/* Allocate driver ring structs */
+	ring = devm_kcalloc(dev, mbox->num_rings, sizeof(*ring), GFP_KERNEL);
+	if (!ring) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	mbox->rings = ring;
+
+	/* Initialize members of driver ring structs */
+	regs = mbox->regs;
+	for (index = 0; index < mbox->num_rings; index++) {
+		ring = &mbox->rings[index];
+		ring->num = index;
+		ring->mbox = mbox;
+		while ((regs < regs_end) &&
+		       (readl_relaxed(regs + RING_VER) != RING_VER_MAGIC))
+			regs += RING_REGS_SIZE;
+		if (regs_end <= regs) {
+			ret = -ENODEV;
+			goto fail;
+		}
+		ring->regs = regs;
+		regs += RING_REGS_SIZE;
+		ring->irq = UINT_MAX;
+		ring->irq_requested = false;
+		ring->msi_timer_val = MSI_TIMER_VAL_MASK;
+		ring->msi_count_threshold = 0x1;
+		ida_init(&ring->requests_ida);
+		memset(ring->requests, 0, sizeof(ring->requests));
+		ring->bd_base = NULL;
+		ring->bd_dma_base = 0;
+		ring->cmpl_base = NULL;
+		ring->cmpl_dma_base = 0;
+		spin_lock_init(&ring->lock);
+		ring->last_pending_msg = NULL;
+		ring->cmpl_read_offset = 0;
+	}
+
+	/* FlexRM is capable of 40-bit physical addresses only */
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
+	if (ret) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret)
+			goto fail;
+	}
+
+	/* Create DMA pool for ring BD memory */
+	mbox->bd_pool = dma_pool_create("bd", dev, RING_BD_SIZE,
+					1 << RING_BD_ALIGN_ORDER, 0);
+	if (!mbox->bd_pool) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	/* Create DMA pool for ring completion memory */
+	mbox->cmpl_pool = dma_pool_create("cmpl", dev, RING_CMPL_SIZE,
+					  1 << RING_CMPL_ALIGN_ORDER, 0);
+	if (!mbox->cmpl_pool) {
+		ret = -ENOMEM;
+		goto fail_destroy_bd_pool;
+	}
+
+	/* Allocate platform MSIs for each ring */
+	ret = platform_msi_domain_alloc_irqs(dev, mbox->num_rings,
+						flexrm_mbox_msi_write);
+	if (ret)
+		goto fail_destroy_cmpl_pool;
+
+	/* Save alloced IRQ numbers for each ring */
+	for_each_msi_entry(desc, dev) {
+		ring = &mbox->rings[desc->platform.msi_index];
+		ring->irq = desc->irq;
+	}
+
+	/* Initialize mailbox controller */
+	mbox->controller.txdone_irq = false;
+	mbox->controller.txdone_poll = true;
+	mbox->controller.txpoll_period = 1;
+	mbox->controller.ops = &flexrm_mbox_chan_ops;
+	mbox->controller.dev = dev;
+	mbox->controller.num_chans = mbox->num_rings;
+	mbox->controller.of_xlate = flexrm_mbox_of_xlate;
+	mbox->controller.chans = devm_kcalloc(dev, mbox->num_rings,
+				sizeof(*mbox->controller.chans), GFP_KERNEL);
+	if (!mbox->controller.chans) {
+		ret = -ENOMEM;
+		goto fail_free_msis;
+	}
+	for (index = 0; index < mbox->num_rings; index++)
+		mbox->controller.chans[index].con_priv = &mbox->rings[index];
+
+	/* Register mailbox controller */
+	ret = mbox_controller_register(&mbox->controller);
+	if (ret)
+		goto fail_free_msis;
+
+	dev_info(dev, "registered flexrm mailbox with %d channels\n",
+			mbox->controller.num_chans);
+
+	return 0;
+
+fail_free_msis:
+	platform_msi_domain_free_irqs(dev);
+fail_destroy_cmpl_pool:
+	dma_pool_destroy(mbox->cmpl_pool);
+fail_destroy_bd_pool:
+	dma_pool_destroy(mbox->bd_pool);
+fail:
+	return ret;
+}
+
+static int flexrm_mbox_remove(struct platform_device *pdev)
+{
+	int index;
+	struct device *dev = &pdev->dev;
+	struct flexrm_ring *ring;
+	struct flexrm_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+
+	platform_msi_domain_free_irqs(dev);
+
+	dma_pool_destroy(mbox->cmpl_pool);
+	dma_pool_destroy(mbox->bd_pool);
+
+	for (index = 0; index < mbox->num_rings; index++) {
+		ring = &mbox->rings[index];
+		ida_destroy(&ring->requests_ida);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id flexrm_mbox_of_match[] = {
+	{ .compatible = "brcm,iproc-flexrm-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, flexrm_mbox_of_match);
+
+static struct platform_driver flexrm_mbox_driver = {
+	.driver = {
+		.name = "brcm-flexrm-mbox",
+		.of_match_table = flexrm_mbox_of_match,
+	},
+	.probe		= flexrm_mbox_probe,
+	.remove		= flexrm_mbox_remove,
+};
+module_platform_driver(flexrm_mbox_driver);
+
+MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
+MODULE_DESCRIPTION("Broadcom FlexRM mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mailbox/brcm-message.h b/include/linux/mailbox/brcm-message.h
index 6b55c93..c20b484 100644
--- a/include/linux/mailbox/brcm-message.h
+++ b/include/linux/mailbox/brcm-message.h
@@ -16,6 +16,7 @@
 
 enum brcm_message_type {
 	BRCM_MESSAGE_UNKNOWN = 0,
+	BRCM_MESSAGE_BATCH,
 	BRCM_MESSAGE_SPU,
 	BRCM_MESSAGE_SBA,
 	BRCM_MESSAGE_MAX,
@@ -23,24 +24,29 @@ enum brcm_message_type {
 
 struct brcm_sba_command {
 	u64 cmd;
+	u64 *cmd_dma;
+	dma_addr_t cmd_dma_addr;
 #define BRCM_SBA_CMD_TYPE_A		BIT(0)
 #define BRCM_SBA_CMD_TYPE_B		BIT(1)
 #define BRCM_SBA_CMD_TYPE_C		BIT(2)
 #define BRCM_SBA_CMD_HAS_RESP		BIT(3)
 #define BRCM_SBA_CMD_HAS_OUTPUT		BIT(4)
 	u64 flags;
-	dma_addr_t input;
-	size_t input_len;
 	dma_addr_t resp;
 	size_t resp_len;
-	dma_addr_t output;
-	size_t output_len;
+	dma_addr_t data;
+	size_t data_len;
 };
 
 struct brcm_message {
 	enum brcm_message_type type;
 	union {
 		struct {
+			struct brcm_message *msgs;
+			unsigned int msgs_queued;
+			unsigned int msgs_count;
+		} batch;
+		struct {
 			struct scatterlist *src;
 			struct scatterlist *dst;
 		} spu;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 0/3] Broadcom FlexRM ring manager support
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel

The Broadcom FlexRM ring manager provides producer-consumer style
ring interface for offload engines on Broadcom iProc SoCs. We can
have one or more instances of Broadcom FlexRM ring manager in a SoC.

This patchset adds a mailbox driver for Broadcom FlexRM ring manager
which can be used by offload engine drivers as mailbox clients.

The Broadcom FlexRM mailbox driver is feature complete for RAID and
Crypto offload engines. We will have incremental patches in-future
for ring-level statistics using debugfs and minor optimizations.

This patchset is based on Linux-4.11-rc4 and it is also available
at flexrm-v7 branch of https://github.com/Broadcom/arm64-linux.git

Changes since v6:
 - Rebased patches for Linux-4.11-rc4
 - Added a patch to remove depends on COMPILE_TEST for kconfig
   option BCM_FLEXRM_MBOX

Changes since v5:
 - Rebased patches for Linux-4.11-rc2
 - Merged all source files into one source file and removed
   driver directory

Changes since v4:
 - Rebased patches for Linux-4.11-rc1

Changes since v3:
 - Fixed mailbox client example DT node in DT bindings document

Changes since v2:
 - Rebased patches for Linux-4.10-rc2

Changes since v1:
 - Use compatile string as brcm,iproc-flexrm-mbox
 - Rephrase commit message and text in DT bindings patch

Anup Patel (3):
  mailbox: Add driver for Broadcom FlexRM ring manager
  dt-bindings: Add DT bindings info for FlexRM ring manager
  mailbox: Remove depends on COMPILE_TEST for BCM_FLEXRM_MBOX

 .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    |   59 +
 drivers/mailbox/Kconfig                            |   11 +
 drivers/mailbox/Makefile                           |    2 +
 drivers/mailbox/bcm-flexrm-mailbox.c               | 1595 ++++++++++++++++++++
 include/linux/mailbox/brcm-message.h               |   14 +-
 5 files changed, 1677 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
 create mode 100644 drivers/mailbox/bcm-flexrm-mailbox.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Youlin Pei @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
	yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <499f9186-82c1-cb5a-91de-754c89bcdd3d-5wv7dgnIgG8@public.gmane.org>

On Fri, 2017-03-24 at 16:22 +0000, Marc Zyngier wrote:
> On 14/02/17 02:56, Youlin Pei wrote:
> > In Mediatek SOCs, the CIRQ is a low power interrupt controller
> > designed to works outside MCUSYS which comprises with Cortex-Ax
> > cores,CCI and GIC.
> > 
> > The CIRQ controller is integrated in between MCUSYS( include
> > Cortex-Ax, CCI and GIC ) and interrupt sources as the second
> > level interrupt controller. The external interrupts which outside
> > MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
> > all edge trigger interupts. When an edge interrupt is triggered,
> > CIRQ can record the status and generate a pulse signal to GIC when
> > flush command executed.
> > 
> > When system enters sleep mode, MCUSYS will be turned off to improve
> > power consumption, also GIC is power down. The edge trigger interrupts
> > will be lost in this scenario without CIRQ.
> > 
> > This commit provides the CIRQ irqchip implement.
> > 
> > Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/irqchip/Makefile       |    2 +-
> >  drivers/irqchip/irq-mtk-cirq.c |  288 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 289 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/irqchip/irq-mtk-cirq.c
> > 
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 0e55d94..db9acd1 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -61,7 +61,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
> >  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
> >  obj-$(CONFIG_KEYSTONE_IRQ)		+= irq-keystone.o
> >  obj-$(CONFIG_MIPS_GIC)			+= irq-mips-gic.o
> > -obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o
> > +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o irq-mtk-cirq.o
> >  obj-$(CONFIG_ARCH_DIGICOLOR)		+= irq-digicolor.o
> >  obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
> >  obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
> > diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> > new file mode 100644
> > index 0000000..b5cf506
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-mtk-cirq.c
> > @@ -0,0 +1,288 @@
> > +/*
> > + * Copyright (c) 2016 MediaTek Inc.
> > + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program 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.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +#include <linux/syscore_ops.h>
> > +
> > +#define CIRQ_ACK	0x40
> > +#define CIRQ_MASK_SET	0xc0
> > +#define CIRQ_MASK_CLR	0x100
> > +#define CIRQ_SENS_SET	0x180
> > +#define CIRQ_SENS_CLR	0x1c0
> > +#define CIRQ_POL_SET	0x240
> > +#define CIRQ_POL_CLR	0x280
> > +#define CIRQ_CONTROL	0x300
> > +
> > +#define CIRQ_EN	0x1
> > +#define CIRQ_EDGE	0x2
> > +#define CIRQ_FLUSH	0x4
> > +
> > +struct mtk_cirq_chip_data {
> > +	void __iomem *base;
> > +	unsigned int ext_irq_start;
> > +	unsigned int ext_irq_end;
> > +	struct irq_domain *domain;
> > +};
> > +
> > +static struct mtk_cirq_chip_data *cirq_data;
> 
> Question in relation to the sysirq patches that have been queued for
> 4.12: Are we going to see something similar with this driver, where
> you're going to have to support multiple non-contiguous register sets?
> I'd rather have this kind of thing from day one, instead of adding that
> at a later time...
> 
> Specially given that the platform that is using this driver does also
> have a sysirq controller.

Hi Marc,
Thanks for your review.

All of the cirq's registers are in continuous space, So no need to
support multiple non-continuous register sets.

> 
> > +
> > +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
> > +{
> > +	struct mtk_cirq_chip_data *chip_data = data->chip_data;
> > +	unsigned int cirq_num = data->hwirq;
> > +	u32 mask = 1 << (cirq_num % 32);
> > +
> > +	writel_relaxed(mask, chip_data->base + offset + (cirq_num / 32) * 4);
> > +}
> > +
> > +static void mtk_cirq_mask(struct irq_data *data)
> > +{
> > +	mtk_cirq_write_mask(data, CIRQ_MASK_SET);
> > +	irq_chip_mask_parent(data);
> > +}
> > +
> > +static void mtk_cirq_unmask(struct irq_data *data)
> > +{
> > +	mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
> > +	irq_chip_unmask_parent(data);
> > +}
> > +
> > +static int mtk_cirq_set_type(struct irq_data *data, unsigned int type)
> > +{
> > +	int ret;
> > +
> > +	switch (type & IRQ_TYPE_SENSE_MASK) {
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_SET);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_SET);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	data = data->parent_data;
> > +	ret = data->chip->irq_set_type(data, type);
> > +	return ret;
> > +}
> > +
> > +static struct irq_chip mtk_cirq_chip = {
> > +	.name			= "MT_CIRQ",
> > +	.irq_mask		= mtk_cirq_mask,
> > +	.irq_unmask		= mtk_cirq_unmask,
> > +	.irq_eoi		= irq_chip_eoi_parent,
> > +	.irq_set_type		= mtk_cirq_set_type,
> > +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> > +#ifdef CONFIG_SMP
> > +	.irq_set_affinity	= irq_chip_set_affinity_parent,
> > +#endif
> > +};
> > +
> > +static int mtk_cirq_domain_translate(struct irq_domain *d,
> > +				     struct irq_fwspec *fwspec,
> > +				     unsigned long *hwirq,
> > +				     unsigned int *type)
> > +{
> > +	if (is_of_node(fwspec->fwnode)) {
> > +		if (fwspec->param_count != 3)
> > +			return -EINVAL;
> > +
> > +		/* No PPI should point to this domain */
> > +		if (fwspec->param[0] != 0)
> > +			return -EINVAL;
> > +
> > +		/* cirq support irq number check */
> > +		if (fwspec->param[1] < cirq_data->ext_irq_start ||
> > +		    fwspec->param[1] > cirq_data->ext_irq_end)
> > +			return -EINVAL;
> > +
> > +		*hwirq = fwspec->param[1] - cirq_data->ext_irq_start;
> > +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int mtk_cirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> > +				 unsigned int nr_irqs, void *arg)
> > +{
> > +	int ret;
> > +	irq_hw_number_t hwirq;
> > +	unsigned int type;
> > +	struct irq_fwspec *fwspec = arg;
> > +	struct irq_fwspec parent_fwspec = *fwspec;
> > +
> > +	ret = mtk_cirq_domain_translate(domain, fwspec, &hwirq, &type);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (WARN_ON(nr_irqs != 1))
> > +		return -EINVAL;
> > +
> > +	irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> > +				      &mtk_cirq_chip,
> > +				      domain->host_data);
> > +
> > +	parent_fwspec.fwnode = domain->parent->fwnode;
> > +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> > +					    &parent_fwspec);
> > +}
> > +
> > +static const struct irq_domain_ops cirq_domain_ops = {
> > +	.translate	= mtk_cirq_domain_translate,
> > +	.alloc		= mtk_cirq_domain_alloc,
> > +	.free		= irq_domain_free_irqs_common,
> > +};
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int mtk_cirq_suspend(void)
> > +{
> > +	u32 value, mask;
> > +	unsigned int irq, hwirq_num;
> > +	bool pending, masked;
> > +	int i, pendret, maskret;
> > +
> > +	hwirq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> > +	for (i = 0; i < hwirq_num; i++) {
> > +		irq = irq_find_mapping(cirq_data->domain, i);
> > +		if (irq) {
> > +			pendret = irq_get_irqchip_state(irq,
> > +							IRQCHIP_STATE_PENDING,
> > +							&pending);
> > +
> > +			maskret = irq_get_irqchip_state(irq,
> > +							IRQCHIP_STATE_MASKED,
> > +							&masked);
> > +
> > +			if (pendret == 0 && maskret == 0 &&
> > +			    (pending && !masked))
> > +				continue;
> > +		}
> > +
> > +		mask = 1 << (i % 32);
> > +		writel_relaxed(mask, cirq_data->base + CIRQ_ACK + (i / 32) * 4);
> > +	}
> 
> It would be worth documenting exactly what this is for. I had to go back
> to our previous discussion in November to remember it.

Okay, i will add some comments to explain what this is for in next
version. Thanks!

> 
> > +
> > +	/* set edge_only mode, record edge-triggerd interrupts */
> > +	/* enable cirq */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	value |= (CIRQ_EDGE | CIRQ_EN);
> > +	writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mtk_cirq_resume(void)
> > +{
> > +	u32 value;
> > +
> > +	/* flush recored interrupts, will send signals to parent controller */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	writel_relaxed(value | CIRQ_FLUSH, cirq_data->base + CIRQ_CONTROL);
> > +
> > +	/* disable cirq */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	value &= ~(CIRQ_EDGE | CIRQ_EN);
> > +	writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> > +}
> > +
> > +static struct syscore_ops mtk_cirq_syscore_ops = {
> > +	.suspend	= mtk_cirq_suspend,
> > +	.resume		= mtk_cirq_resume,
> > +};
> > +
> > +static void mtk_cirq_syscore_init(void)
> > +{
> > +	register_syscore_ops(&mtk_cirq_syscore_ops);
> > +}
> > +#else
> > +static inline void mtk_cirq_syscore_init(void) {}
> > +#endif
> > +
> > +static int __init mtk_cirq_of_init(struct device_node *node,
> > +				   struct device_node *parent)
> > +{
> > +	struct irq_domain *domain, *domain_parent;
> > +	unsigned int irq_num;
> > +	int ret;
> > +
> > +	domain_parent = irq_find_host(parent);
> > +	if (!domain_parent) {
> > +		pr_err("mtk_cirq: interrupt-parent not found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	cirq_data = kzalloc(sizeof(*cirq_data), GFP_KERNEL);
> > +	if (!cirq_data)
> > +		return -ENOMEM;
> > +
> > +	cirq_data->base = of_iomap(node, 0);
> > +	if (!cirq_data->base) {
> > +		pr_err("mtk_cirq: unable to map cirq register\n");
> > +		ret = -ENXIO;
> > +		goto out_free;
> > +	}
> > +
> > +	ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 0,
> > +					 &cirq_data->ext_irq_start);
> > +	if (ret)
> > +		goto out_unmap;
> > +
> > +	ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 1,
> > +					 &cirq_data->ext_irq_end);
> > +	if (ret)
> > +		goto out_unmap;
> > +
> > +	irq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> > +	domain = irq_domain_add_hierarchy(domain_parent, 0,
> > +					  irq_num, node,
> > +					  &cirq_domain_ops, cirq_data);
> > +	if (!domain) {
> > +		ret = -ENOMEM;
> > +		goto out_unmap;
> > +	}
> > +	cirq_data->domain = domain;
> > +
> > +	mtk_cirq_syscore_init();
> > +
> > +	return 0;
> > +
> > +out_unmap:
> > +	iounmap(cirq_data->base);
> > +out_free:
> > +	kfree(cirq_data);
> > +	return ret;
> > +}
> > +
> > +IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
> > 
> 
> It otherwise looks much better than the previous iteration.
> 
> Thanks,
> 
> 	M.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC
From: Eduardo Valentin @ 2017-03-29  4:58 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
	Rob Herring, Frank Rowand, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1489356665-3175-7-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>

On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> Add basic thermal driver for bcm2835 SoC.
> 
> This driver currently make sure that tsense HW block is set up
> correctly.
> 
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

I see no issue with this driver at this point.

As I mentioned, the full series needs to go together. You either get an
Ack from OF maintainers, and I take the series, or you can add my

Acked-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

after fixing the small comment from Nobuhiro.

> ---
>  drivers/thermal/Kconfig           |    8 +
>  drivers/thermal/Makefile          |    1 +
>  drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 324 insertions(+)
>  create mode 100644 drivers/thermal/bcm2835_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b343..3bd2406 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>  	  the primitive temperature sensor embedded in zx2967 SoCs.
>  	  This sensor generates the real time die temperature.
>  
> +config BCM2835_THERMAL
> +	tristate "Thermal sensors on bcm2835 SoC"
> +	depends on ARCH_BCM2835 || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on THERMAL_OF
> +	help
> +	  Support for thermal sensors on Broadcom bcm2835 SoCs.

small suggestion here:

-If you are sending a new version fixing Nobuhiro comment, you could take
the opportunity to improve the driver Kconfig help description.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support
From: Eduardo Valentin @ 2017-03-29  4:54 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
	Rob Herring, Frank Rowand, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1489356665-3175-3-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>


On Sun, Mar 12, 2017 at 10:11:01PM +0000, Stefan Wahren wrote:
> Use the new function of_property_read_s32_array() to prepare
> of-thermal for negative coefficients. These are used by
> the upcoming bcm2835_thermal driver.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  drivers/thermal/of-thermal.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d04ec3b..491d58a 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	struct device_node *child = NULL, *gchild;
>  	struct __thermal_zone *tz;
>  	int ret, i;
> -	u32 prop, coef[2];
> +	u32 prop;
> +	s32 coef[2];
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	 * one sensor per thermal zone. Thus, we are considering
>  	 * only the first two values as slope and offset.
>  	 */
> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);

For the scope of the proposed change I see not problem with this patch.

However, this needs to go in after the of core change gets accepted.
Alternatively, I could take the full series if you get an Ack from OF
maintainers.

>  	if (ret == 0) {
>  		tz->slope = coef[0];
>  		tz->offset = coef[1];
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH] of: Fix DMA configuration for non-DT masters
From: Oza Oza @ 2017-03-29  4:53 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Linux IOMMU, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <67d1f46172599be243405f4341665fd0ef9ab969.1490726288.git.robin.murphy-5wv7dgnIgG8@public.gmane.org>

On Wed, Mar 29, 2017 at 12:27 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> For PCI masters not represented in DT, we pass the OF node of their
> associated host bridge to of_dma_configure(), such that they can inherit
> the appropriate DMA configuration from whatever is described there.
> Unfortunately, whilst this has worked for the "dma-coherent" property,
> it turns out to miss the case where the host bridge node has a non-empty
> "dma-ranges", since nobody is expecting the 'device' to be a bus itself.
>
> It transpires, though, that the de-facto interface since the prototype
> change in 1f5c69aa51f9 ("of: Move of_dma_configure() to device.c to help
> re-use") is very clear-cut: either the master_np argument is redundant
> with dev->of_node, or dev->of_node is NULL and master_np is the relevant
> parent bus. Let's ratify that behaviour, then teach the whole
> of_dma_configure() pipeline to cope with both cases properly.
>
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>
> This is what I'd consider the better fix - rather than adding yet more
> special cases - which will also make it simple to handle multiple
> "dma-ranges" entries with minimal further disruption. The callers now
> left passing dev->of_node as 'parent' are harmless, but look a bit silly
> and clearly want cleaning up - I'd be partial to renaming the existing
> function and having a single-argument wrapper for the 'normal' case, e.g.:
>
> static inline int of_dma_configure(struct device_node *dev)
> {
>         return of_dma_configure_parent(dev, NULL);
> }
>
> Thoughts?
>
> Robin.
>
>  drivers/iommu/of_iommu.c   |  7 ++++---
>  drivers/of/address.c       | 37 +++++++++++++++++++++++++------------
>  drivers/of/device.c        | 12 +++++++-----
>  include/linux/of_address.h |  7 ++++---
>  include/linux/of_device.h  |  4 ++--
>  include/linux/of_iommu.h   |  4 ++--
>  6 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 2683e9fc0dcf..35aff07bb5eb 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -138,21 +138,22 @@ static const struct iommu_ops
>  }
>
>  const struct iommu_ops *of_iommu_configure(struct device *dev,
> -                                          struct device_node *master_np)
> +                                          struct device_node *parent)
>  {
>         struct of_phandle_args iommu_spec;
> -       struct device_node *np;
> +       struct device_node *np, *master_np;
>         const struct iommu_ops *ops = NULL;
>         int idx = 0;
>
>         if (dev_is_pci(dev))
> -               return of_pci_iommu_configure(to_pci_dev(dev), master_np);
> +               return of_pci_iommu_configure(to_pci_dev(dev), parent);
>
>         /*
>          * We don't currently walk up the tree looking for a parent IOMMU.
>          * See the `Notes:' section of
>          * Documentation/devicetree/bindings/iommu/iommu.txt
>          */
> +       master_np = dev->of_node ? dev->of_node : parent;
>         while (!of_parse_phandle_with_args(master_np, "iommus",
>                                            "#iommu-cells", idx,
>                                            &iommu_spec)) {
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..833bc17f5e55 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -808,6 +808,7 @@ EXPORT_SYMBOL(of_io_request_and_map);
>  /**
>   * of_dma_get_range - Get DMA range info
>   * @np:                device node to get DMA range info
> + * @parent:    node of device's parent bus, if @np is NULL
>   * @dma_addr:  pointer to store initial DMA address of DMA range
>   * @paddr:     pointer to store initial CPU address of DMA range
>   * @size:      pointer to store size of DMA range
> @@ -822,36 +823,48 @@ EXPORT_SYMBOL(of_io_request_and_map);
>   * It returns -ENODEV if "dma-ranges" property was not found
>   * for this device in DT.
>   */
> -int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
> +int of_dma_get_range(struct device_node *np, struct device_node *parent,
> +                    u64 *dma_addr, u64 *paddr, u64 *size)
>  {
> -       struct device_node *node = of_node_get(np);
> +       struct device_node *node;
>         const __be32 *ranges = NULL;
>         int len, naddr, nsize, pna;
>         int ret = 0;
>         u64 dmaaddr;
>
> -       if (!node)
> -               return -EINVAL;
> -
> -       while (1) {
> +       if (np) {
> +               node = of_node_get(np);
>                 naddr = of_n_addr_cells(node);
>                 nsize = of_n_size_cells(node);
>                 node = of_get_next_parent(node);
> -               if (!node)
> -                       break;
> +       } else if (parent) {
> +               node = of_node_get(parent);
> +               np = parent;
> +               if (of_property_read_u32(node, "#address-cells", &naddr))
> +                       naddr = of_n_addr_cells(node);
> +               if (of_property_read_u32(node, "#size-cells", &nsize))
> +                       nsize = of_n_size_cells(node);
> +       } else {
> +               return -EINVAL;
> +       }
>
> +       while (node) {
>                 ranges = of_get_property(node, "dma-ranges", &len);
>
> -               /* Ignore empty ranges, they imply no translation required */
> -               if (ranges && len > 0)
> -                       break;
> -
>                 /*
>                  * At least empty ranges has to be defined for parent node if
>                  * DMA is supported
>                  */
>                 if (!ranges)
>                         break;
> +
> +               /* Ignore empty ranges, they imply no translation required */
> +               if (len > 0)
> +                       break;
> +
> +               naddr = of_n_addr_cells(node);
> +               nsize = of_n_size_cells(node);
> +               node = of_get_next_parent(node);
>         }
>
>         if (!ranges) {
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 9bb8518b28f2..57ec5324ed6c 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -73,7 +73,8 @@ int of_device_add(struct platform_device *ofdev)
>  /**
>   * of_dma_configure - Setup DMA configuration
>   * @dev:       Device to apply DMA configuration
> - * @np:                Pointer to OF node having DMA configuration
> + * @parent:    OF node of device's parent bus, if @dev is not
> + *             represented in DT (i.e. @dev->of_node is NULL)
>   *
>   * Try to get devices's DMA configuration from DT and update it
>   * accordingly.
> @@ -82,13 +83,14 @@ int of_device_add(struct platform_device *ofdev)
>   * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
>   * to fix up DMA configuration.
>   */
> -void of_dma_configure(struct device *dev, struct device_node *np)
> +void of_dma_configure(struct device *dev, struct device_node *parent)
>  {
>         u64 dma_addr, paddr, size;
>         int ret;
>         bool coherent;
>         unsigned long offset;
>         const struct iommu_ops *iommu;
> +       struct device_node *np = dev->of_node;
>
>         /*
>          * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
> @@ -104,7 +106,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>         if (!dev->dma_mask)
>                 dev->dma_mask = &dev->coherent_dma_mask;
>
> -       ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> +       ret = of_dma_get_range(np, parent, &dma_addr, &paddr, &size);
>         if (ret < 0) {
>                 dma_addr = offset = 0;
>                 size = dev->coherent_dma_mask + 1;
> @@ -132,11 +134,11 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>         dev->coherent_dma_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
>         *dev->dma_mask = dev->coherent_dma_mask;
>
> -       coherent = of_dma_is_coherent(np);
> +       coherent = of_dma_is_coherent(np ? np : parent);
>         dev_dbg(dev, "device is%sdma coherent\n",
>                 coherent ? " " : " not ");
>
> -       iommu = of_iommu_configure(dev, np);
> +       iommu = of_iommu_configure(dev, parent);
>         dev_dbg(dev, "device is%sbehind an iommu\n",
>                 iommu ? " " : " not ");
>
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 37864734ca50..f1a507f3ae57 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -52,8 +52,8 @@ extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
>  extern struct of_pci_range *of_pci_range_parser_one(
>                                         struct of_pci_range_parser *parser,
>                                         struct of_pci_range *range);
> -extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
> -                               u64 *paddr, u64 *size);
> +extern int of_dma_get_range(struct device_node *np, struct device_node *parent,
> +                               u64 *dma_addr, u64 *paddr, u64 *size);
>  extern bool of_dma_is_coherent(struct device_node *np);
>  #else /* CONFIG_OF_ADDRESS */
>  static inline void __iomem *of_io_request_and_map(struct device_node *device,
> @@ -95,7 +95,8 @@ static inline struct of_pci_range *of_pci_range_parser_one(
>         return NULL;
>  }
>
> -static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
> +static inline int of_dma_get_range(struct device_node *np,
> +                               struct device_node *parent, u64 *dma_addr,
>                                 u64 *paddr, u64 *size)
>  {
>         return -ENODEV;
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index c12dace043f3..bcd2b6fbeef3 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>         return of_node_get(cpu_dev->of_node);
>  }
>
> -void of_dma_configure(struct device *dev, struct device_node *np);
> +void of_dma_configure(struct device *dev, struct device_node *parent);
>  #else /* CONFIG_OF */
>
>  static inline int of_driver_match_device(struct device *dev,
> @@ -103,7 +103,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>  {
>         return NULL;
>  }
> -static inline void of_dma_configure(struct device *dev, struct device_node *np)
> +static inline void of_dma_configure(struct device *dev, struct device_node *parent)
>  {}
>  #endif /* CONFIG_OF */
>
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index 13394ac83c66..c02b62e8e6ed 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -12,7 +12,7 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
>                              size_t *size);
>
>  extern const struct iommu_ops *of_iommu_configure(struct device *dev,
> -                                       struct device_node *master_np);
> +                                       struct device_node *parent);
>
>  #else
>
> @@ -24,7 +24,7 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
>  }
>
>  static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
> -                                        struct device_node *master_np)
> +                                        struct device_node *parent)
>  {
>         return NULL;
>  }
> --
> 2.11.0.dirty
>

pci and memory mapped device world is different. of course if you talk
from iommu perspective, all the master are same for IOMMU

but the original intention of the patch is to try to solve 2 problems.
please refer to https://lkml.org/lkml/2017/3/29/10

1) expose generic API for pci world clients to configure their
dma-ranges. right now there is none.
2) same API can be used by IOMMU to derive dma_mask.

while the current patch posted to handle dma-ranges for both memory
mapped and pci devices, which I think is overdoing.

besides we have different configuration of dma-ranges based on iommu
is enabled or not enabled.
 #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
0x80000000 0x00 0x80000000 \
                                      0x43000000 0x08 0x00000000 0x08
0x00000000 0x08 0x00000000 \
                                      0x43000000 0x80 0x00000000 0x80
0x00000000 0x40 0x00000000>;
Not sure if this patch will consider above dma-ranges.

my suggestion is to leave existing of_dma_get_range as is, and have
new function for pci world as discussed in
please refer to https://lkml.org/lkml/2017/3/29/10

Regards,
Oza.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support
From: Eduardo Valentin @ 2017-03-29  4:52 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
	Rob Herring, Frank Rowand, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1489356665-3175-3-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>

Stefen,

On Sun, Mar 12, 2017 at 10:11:01PM +0000, Stefan Wahren wrote:
> Use the new function of_property_read_s32_array() to prepare
> of-thermal for negative coefficients. These are used by
> the upcoming bcm2835_thermal driver.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>

Once done with the comment pointed by Frank, you can add my
Reviewed-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/thermal/of-thermal.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d04ec3b..491d58a 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	struct device_node *child = NULL, *gchild;
>  	struct __thermal_zone *tz;
>  	int ret, i;
> -	u32 prop, coef[2];
> +	u32 prop;
> +	s32 coef[2];
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	 * one sensor per thermal zone. Thus, we are considering
>  	 * only the first two values as slope and offset.
>  	 */
> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
>  	if (ret == 0) {
>  		tz->slope = coef[0];
>  		tz->offset = coef[1];
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 1/3] of/pci: dma-ranges to account highest possible host bridge dma_mask
From: Oza Oza @ 2017-03-29  4:43 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Rob Herring, Joerg Roedel, Linux IOMMU,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org
In-Reply-To: <de6bfedf-62aa-98e4-dae5-7862ab3ba754-5wv7dgnIgG8@public.gmane.org>

On Tue, Mar 28, 2017 at 7:59 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 28/03/17 06:27, Oza Oza wrote:
>> On Mon, Mar 27, 2017 at 8:16 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Sat, Mar 25, 2017 at 12:31 AM, Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>> it is possible that PCI device supports 64-bit DMA addressing,
>>>> and thus it's driver sets device's dma_mask to DMA_BIT_MASK(64),
>>>> however PCI host bridge may have limitations on the inbound
>>>> transaction addressing. As an example, consider NVME SSD device
>>>> connected to iproc-PCIe controller.
>>>>
>>>> Currently, the IOMMU DMA ops only considers PCI device dma_mask
>>>> when allocating an IOVA. This is particularly problematic on
>>>> ARM/ARM64 SOCs where the IOMMU (i.e. SMMU) translates IOVA to
>>>> PA for in-bound transactions only after PCI Host has forwarded
>>>> these transactions on SOC IO bus. This means on such ARM/ARM64
>>>> SOCs the IOVA of in-bound transactions has to honor the addressing
>>>> restrictions of the PCI Host.
>>>>
>>>> current pcie frmework and of framework integration assumes dma-ranges
>>>> in a way where memory-mapped devices define their dma-ranges.
>>>> dma-ranges: (child-bus-address, parent-bus-address, length).
>>>>
>>>> but iproc based SOCs and even Rcar based SOCs has PCI world dma-ranges.
>>>> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
>>>
>>> If you implement a common function, then I expect to see other users
>>> converted to use it. There's also PCI hosts in arch/powerpc that parse
>>> dma-ranges.
>>
>> the common function should be similar to what
>> of_pci_get_host_bridge_resources is doing right now.
>> it parses ranges property right now.
>>
>> the new function would look look following.
>>
>> of_pci_get_dma_ranges(struct device_node *dev, struct list_head *resources)
>> where resources would return the dma-ranges.
>>
>> but right now if you see the patch, of_dma_configure calls the new
>> function, which actually returns the largest possible size.
>> so this new function has to be generic in a way where other PCI hosts
>> can use it. but certainly iproc(Broadcom SOC) , rcar based SOCs can
>> use it for sure.
>>
>> although having powerpc using it;  is a separate exercise, since I do
>> not have any access to other PCI hosts such as powerpc. but we can
>> workout with them on thsi forum if required.
>>
>> so overall, of_pci_get_dma_ranges has to serve following 2 purposes.
>>
>> 1) it has to return largest possible size to of_dma_configure to
>> generate largest possible dma_mask.
>>
>> 2) it also has to return resources (dma-ranges) parsed, to the users.
>>
>> so to address above needs
>>
>> of_pci_get_dma_ranges(struct device_node *dev, struct list_head
>> *resources, u64 *size)
>>
>> dev -> device node.
>> resources -> dma-ranges in allocated list.
>> size -> highest possible size to generate possible dma_mask for
>> of_dma_configure.
>>
>> let em know how this sounds.
>
> Note that the point of passing PCI host bridges into of_dma_configure()
> in the first place was to avoid having some separate PCI-specific path
> for DMA configuration. I worry that introducing bus-specific dma-ranges
> parsing largely defeats that, since we end up with the worst of both
> worlds; effectively-duplicated code, and/or a load of extra complexity
> to then attempt to reconverge the divergent paths (there really
> shouldn't be any need to allocate a list of anything). Given that
> of_translate_dma_address() is already bus-agnostic, it hardly seems
> justifiable for its caller not to be so as well, especially when it
> mostly just comes down to getting the right #address-cells value.
>
> The patch below is actually enough to make typical cases work, but is
> vile, so I'm not seriously considering it (hence I've not bothered
> making IOMMU configuration handle all circumstances). What it has served
> to do, though, is give me a clear idea of how to properly sort out the
> not-quite-right device/parent assumptions between of_dma_configure() and
> of_dma_get_range() rather than bodging around them any further - stay tuned.
>
> Robin.
>
> ----->8-----
> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Subject: [PATCH] of/pci: Use child node for DMA configuration
>
> of_dma_configure() expects to be passed an OF node representing the
> device being configured - for PCI devices we currently pass the node of
> the appropriate host controller, which sort of works for inherited
> properties which may appear at any level, like "dma-coherent", but falls
> apart for properties which actually care about specific device-parent
> relationships, like "dma-ranges".
>
> Solve this by attempting to find a suitable child node if the PCI
> hierarchy is actually represented in DT, and if not then faking one up
> as a last resort, to make all of DMA configuration work as expected.
>
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/of_iommu.c |  3 ++-
>  drivers/pci/of.c         | 24 ++++++++++++++++++++++++
>  drivers/pci/probe.c      | 14 +++++++++++++-
>  include/linux/pci.h      |  3 +++
>  4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 2683e9fc0dcf..35c97b945c15 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -146,7 +146,8 @@ const struct iommu_ops *of_iommu_configure(struct
> device *dev,
>         int idx = 0;
>
>         if (dev_is_pci(dev))
> -               return of_pci_iommu_configure(to_pci_dev(dev), master_np);
> +               return of_pci_iommu_configure(to_pci_dev(dev),
> +                                             of_get_next_parent(master_np));
>
>         /*
>          * We don't currently walk up the tree looking for a parent IOMMU.
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index e112da11630e..96eece1c423d 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -88,3 +88,27 @@ struct irq_domain
> *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
>         return NULL;
>  #endif
>  }
> +
> +struct device_node *pci_dev_get_dma_of_node(struct pci_dev *dev)
> +{
> +       struct device_node *np;
> +       struct pci_bus *bus = dev->bus;
> +
> +       /* Is the device itself actually described in the DT? */
> +       np = of_node_get(dev->dev.of_node);
> +       if (np)
> +               return np;
> +
> +       /* Or is some intermediate bridge? That would do... */
> +       for (bus = dev->bus; bus->parent; bus = bus->parent) {
> +               np = of_node_get(bus->bridge->of_node);
> +               if (np)
> +                       return np;
> +       }
> +
> +       /* Failing that, any child of the same host bridge? */
> +       if (bus->bridge->parent)
> +               np = of_get_next_child(bus->bridge->parent->of_node, NULL);
> +
> +       return np;
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dfc9a2794141..4f7ade64aa3e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1906,7 +1906,19 @@ static void pci_dma_configure(struct pci_dev *dev)
>
>         if (IS_ENABLED(CONFIG_OF) &&
>                 bridge->parent && bridge->parent->of_node) {
> -                       of_dma_configure(&dev->dev, bridge->parent->of_node);
> +               struct device_node *np, tmp = {0};
> +
> +               np = pci_dev_get_dma_of_node(dev);
> +               if (!np) {
> +                       np = &tmp;
> +                       of_node_set_flag(np, OF_DETACHED);
> +                       of_node_init(np);
> +                       tmp.parent = bridge->parent->of_node;
> +               }
> +
> +               of_dma_configure(&dev->dev, np);
> +               if (np != &tmp)
> +                       of_node_put(np);
>         } else if (has_acpi_companion(bridge)) {
>                 struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>                 enum dev_dma_attr attr = acpi_get_dma_attr(adev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index eb3da1a04e6c..94ecd1817f58 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2086,6 +2086,7 @@ void pci_release_of_node(struct pci_dev *dev);
>  void pci_set_bus_of_node(struct pci_bus *bus);
>  void pci_release_bus_of_node(struct pci_bus *bus);
>  struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus);
> +struct device_node *pci_dev_get_dma_of_node(struct pci_dev *dev);
>
>  /* Arch may override this (weak) */
>  struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
> @@ -2110,6 +2111,8 @@ static inline struct device_node *
>  pci_device_to_OF_node(const struct pci_dev *pdev) { return NULL; }
>  static inline struct irq_domain *
>  pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
> +static inline struct device_node *
> +pci_dev_get_dma_of_node(struct pci_dev *dev) { return NULL; }
>  #endif  /* CONFIG_OF */
>
>  #ifdef CONFIG_ACPI
>

pci and memory mapped device world is different. of course if you talk
from iommu perspective, all the master are same for IOMMU
but the original intention of the patch is to try to solve 2 problems.

1) expose generic API for pci world clients to configure their
dma-ranges. right now there is none.
2) same API can be used by IOMMU to derive dma_mask.

the implementation tries to handle dma-ranges for both memory mapped
and pci devices, which I think is overkill.
besides we have different configuration of dma-ranges based on iommu
is enabled or not enabled.

 #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
0x80000000 0x00 0x80000000 \
                                      0x43000000 0x08 0x00000000 0x08
0x00000000 0x08 0x00000000 \
                                      0x43000000 0x80 0x00000000 0x80
0x00000000 0x40 0x00000000>;


the of_dma_get_range is written with respect to traditional memory
ranges, they were not meant for pci dma-ranges.

Regards,
Oza.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 2/3] mailbox: Add iProc mailbox controller driver
From: Jassi Brar @ 2017-03-29  4:36 UTC (permalink / raw)
  To: Jonathan Richardson
  Cc: Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	Russell King, Vikram Prakash, Devicetree List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	BCM Kernel Feedback
In-Reply-To: <0e8ac72a-6b2f-171d-41e7-7a6cefc4c7c4-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Tue, Mar 28, 2017 at 11:00 PM, Jonathan Richardson
<jonathan.richardson-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 17-03-15 10:30 AM, Jassi Brar wrote:

>> The right way to do is have a 'server/owner' client that accepts
>> requests from various clients and serially queue them to mailbox. That
>> way you can keep the controller driver free from quirks (like max wait
>> time of 30us) of your present platform.
>
> Do you mean 1 mailbox client with one mailbox channel that all the mbox client drivers share?
> I thought this would work when I suggested it previously but the client callbacks are necessary
> in all txdone modes. Client drivers that send the messages need the callbacks
>
That's a legit requirement.

> and this is only possible with multiple mbox clients.
>
That is incorrect.
It is trivial to support callbacks to end clients from your common
code... just ask for callback along with the message to be submitted
to mailbox api.

> And a channel can only have 1 mbox client. Clients in multiple drivers need the callbacks to either know when to start polling, or be notified when the transaction is complete. It would be nice if multiple clients could use the same channel.
>
We had to choose from shared vs exclusive access to channels... latter
was chosen because there are ways to still support former.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: vf610-zii-dev-rev-b: change switch2 label
From: Shawn Guo @ 2017-03-29  3:49 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Florian Fainelli,
	Andrew Lunn
In-Reply-To: <20170328192426.13798-1-vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>

On Tue, Mar 28, 2017 at 03:24:26PM -0400, Vivien Didelot wrote:
> Rename the switch2@0 label of the switch2 node to switch@0 to respect
> the general unit@address DTS rule, and be consistent with the other
> switch nodes of the DTS file.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx6qp-nitrogen6_som2: add Quad Plus variant of the SOM
From: Shawn Guo @ 2017-03-29  3:44 UTC (permalink / raw)
  To: Gary Bisson
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20170327160601.12871-1-gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

On Mon, Mar 27, 2017 at 06:06:01PM +0200, Gary Bisson wrote:
> https://boundarydevices.com/product/nit6x-som-v2/
> 
> Signed-off-by: Gary Bisson <gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

Please post to LAKML.

Shawn

> ---
>  arch/arm/boot/dts/Makefile                  |  1 +
>  arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts | 55 +++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..6190afe94371 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -426,6 +426,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6q-wandboard.dtb \
>  	imx6q-wandboard-revb1.dtb \
>  	imx6qp-nitrogen6_max.dtb \
> +	imx6qp-nitrogen6_som2.dtb \
>  	imx6qp-sabreauto.dtb \
>  	imx6qp-sabresd.dtb
>  dtb-$(CONFIG_SOC_IMX6SL) += \
> diff --git a/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts b/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts
> new file mode 100644
> index 000000000000..011726c836cd
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2017 Boundary Devices, Inc.
> + *
> + * 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 "imx6qp.dtsi"
> +#include "imx6qdl-nitrogen6_som2.dtsi"
> +
> +/ {
> +	model = "Boundary Devices i.MX6 Quad Plus Nitrogen6_SOM2 Board";
> +	compatible = "boundary,imx6qp-nitrogen6_som2", "fsl,imx6qp";
> +};
> +
> +&sata {
> +	status = "okay";
> +};
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 00/15] ARM: dts: i.MX6: Add lcdif/sound nodes on Engicam SOMs
From: Shawn Guo @ 2017-03-29  3:40 UTC (permalink / raw)
  To: Jagan Teki
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer, Fabio Estevam,
	Matteo Lisi, Michael Trimarchi, Jagan Teki
In-Reply-To: <1490372330-5611-1-git-send-email-jagan-oRp2ZoJdM/RWk0Htik3J/w@public.gmane.org>

On Fri, Mar 24, 2017 at 09:48:35PM +0530, Jagan Teki wrote:
> Jagan Teki (15):
>   ARM: dts: imx6ul-geam: Add backlight support for lcdif
>   ARM: dts: imx6ul-isiot: Add backlight support for lcdif
>   ARM: dts: imx6ul-isiot: Add imx6ul-isiot-common.dtsi
>   ARM: dts: imx6ul-isiot: Add i2c nodes
>   ARM: dts: imx6ul-isiot-common: Add touchscreen node
>   ARM: dts: imx6ul-[geam|isiot]: Add sai2 node

Applied above ones.

Shawn

>   ARM: dts: imx6ul-isiot: Add Sound card with codec node
>   ARM: dts: imx6ul-geam: Add Sound card with codec node
>   ARM: dts: imx6qdl-icore: Add Sound card with codec node
>   ARM: dts: imx6dl-icore-rqs: Add sgtl5000 codec node
>   ARM: dts: imx6qdl-icore-rqs: Replace codec 'lable: node@addr'
>   ARM: dts: imx6dl-icore: Add LVDS support
>   ARM: dts: imx6dl-icore: Add touchscreen node
>   ARM: dts: imx6qdl-icore-rqs: Add CAN nodes
>   ARM: dts: imx6ul-isiot: Add FEC node support
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 10/15] ARM: dts: imx6dl-icore-rqs: Add sgtl5000 codec node
From: Shawn Guo @ 2017-03-29  3:29 UTC (permalink / raw)
  To: Jagan Teki
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer, Fabio Estevam,
	Matteo Lisi, Michael Trimarchi, Jagan Teki
In-Reply-To: <1490372330-5611-11-git-send-email-jagan-oRp2ZoJdM/RWk0Htik3J/w@public.gmane.org>

On Fri, Mar 24, 2017 at 09:48:45PM +0530, Jagan Teki wrote:
> From: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> 
> - Moved sound card node on imx6qdl-icore-rqs.dtsi
> - Added codec node in imx6dl-icore-rqs from imx6q-icore-rqs
> 
> Cc: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Matteo Lisi <matteo.lisi-4s7YQHO/iPVBDgjK7y7TUQ@public.gmane.org>
> Cc: Michael Trimarchi <michael-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> ---
> Changes for v2:
> - Newly added patch
> 
>  arch/arm/boot/dts/imx6dl-icore-rqs.dts   | 11 +++++++++++
>  arch/arm/boot/dts/imx6q-icore-rqs.dts    | 13 -------------
>  arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi | 13 +++++++++++++
>  3 files changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6dl-icore-rqs.dts b/arch/arm/boot/dts/imx6dl-icore-rqs.dts
> index cf42c2f..5b4c14c 100644
> --- a/arch/arm/boot/dts/imx6dl-icore-rqs.dts
> +++ b/arch/arm/boot/dts/imx6dl-icore-rqs.dts
> @@ -49,3 +49,14 @@
>  	model = "Engicam i.CoreM6 DualLite/Solo RQS Starter Kit";
>  	compatible = "engicam,imx6-icore-rqs", "fsl,imx6dl";
>  };
> +
> +&i2c3 {
> +	codec: sgtl5000@0a {

Why adding something that the following patch will change right away?

Shawn

> +		compatible = "fsl,sgtl5000";
> +		reg = <0x0a>;
> +		clocks = <&clks IMX6QDL_CLK_CKO>;
> +		VDDA-supply = <&reg_2p5v>;
> +		VDDIO-supply = <&reg_3p3v>;
> +		VDDD-supply = <&reg_1p8v>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/imx6q-icore-rqs.dts b/arch/arm/boot/dts/imx6q-icore-rqs.dts
> index e451b4c..441a28e5 100644
> --- a/arch/arm/boot/dts/imx6q-icore-rqs.dts
> +++ b/arch/arm/boot/dts/imx6q-icore-rqs.dts
> @@ -47,19 +47,6 @@
>  / {
>  	model = "Engicam i.CoreM6 Quad/Dual RQS Starter Kit";
>  	compatible = "engicam,imx6-icore-rqs", "fsl,imx6q";
> -
> -	sound {
> -		compatible = "fsl,imx-audio-sgtl5000";
> -		model = "imx-audio-sgtl5000";
> -		ssi-controller = <&ssi1>;
> -		audio-codec = <&codec>;
> -		audio-routing =
> -			"MIC_IN", "Mic Jack",
> -			"Mic Jack", "Mic Bias",
> -			"Headphone Jack", "HP_OUT";
> -		mux-int-port = <1>;
> -		mux-ext-port = <4>;
> -	};
>  };
>  
>  &i2c3 {
> diff --git a/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi b/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
> index 5fab5be..4032d38 100644
> --- a/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
> @@ -118,6 +118,19 @@
>  		clocks = <&clks IMX6QDL_CLK_LVDS2_GATE>;
>  		clock-names = "refclk";
>  	};
> +
> +	sound {
> +		compatible = "fsl,imx-audio-sgtl5000";
> +		model = "imx-audio-sgtl5000";
> +		ssi-controller = <&ssi1>;
> +		audio-codec = <&codec>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",
> +			"Mic Jack", "Mic Bias",
> +			"Headphone Jack", "HP_OUT";
> +		mux-int-port = <1>;
> +		mux-ext-port = <4>;
> +	};
>  };
>  
>  &clks {
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 07/15] ARM: dts: imx6ul-isiot: Add Sound card with codec node
From: Shawn Guo @ 2017-03-29  3:27 UTC (permalink / raw)
  To: Jagan Teki
  Cc: linux-arm-kernel, devicetree, linux-kernel, Sascha Hauer,
	Fabio Estevam, Matteo Lisi, Michael Trimarchi, Jagan Teki
In-Reply-To: <1490372330-5611-8-git-send-email-jagan@openedev.com>

On Fri, Mar 24, 2017 at 09:48:42PM +0530, Jagan Teki wrote:
> From: Jagan Teki <jagan@amarulasolutions.com>
> 
> Add support for Sound card and related codec(via i2c1) nodes
> on Engicam Is.IoT MX6UL variant module boards.
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

<snip>

> diff --git a/arch/arm/boot/dts/imx6ul-isiot.dtsi b/arch/arm/boot/dts/imx6ul-isiot.dtsi
> index ea30380..9648de9 100644
> --- a/arch/arm/boot/dts/imx6ul-isiot.dtsi
> +++ b/arch/arm/boot/dts/imx6ul-isiot.dtsi
> @@ -69,6 +69,35 @@
>  				    100>;
>  		default-brightness-level = <100>;
>  	};
> +
> +	reg_1p8v: regulator-1p8v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "1P8V";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3P3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +
> +	sound {
> +		compatible = "fsl,imx-audio-sgtl5000";
> +		model = "imx6ul-isiot-sgtl5000";
> +		cpu-dai = <&sai2>;
> +		audio-codec = <&sgtl5000>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",
> +			"Mic Jack", "Mic Bias",
> +			"Headphone Jack", "HP_OUT";
> +	};

Can you consider to use simple-audio-card bindings instead?

Shawn

>  };
>  
>  &i2c1 {
> -- 
> 1.9.1
> 

^ permalink raw reply

* Re: [PATCH v3 1/5] dt-bindings: Document STM32 IWDG bindings
From: Guenter Roeck @ 2017-03-29  3:15 UTC (permalink / raw)
  To: Yannick Fertre, Wim Van Sebroeck, Rob Herring, Alexandre TORGUE,
	Benjamin Gaignard, Maxime Coquelin
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Philippe Cornu,
	Gabriel FERNANDEZ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490195083-25117-2-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>

On 03/22/2017 08:04 AM, Yannick Fertre wrote:
> From: Yannick Fertre <yfertre.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This adds documentation of device tree bindings for the STM32 IWDG
> (Independent WatchDoG).
>
> Change-Id: Idadacc806d00205fe9af2e8606af229b4b760558

Please drop.

> Signed-off-by: Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> ---
>  .../devicetree/bindings/watchdog/st,stm32-iwdg.txt        | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
> new file mode 100644
> index 0000000..036b040
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
> @@ -0,0 +1,15 @@
> +STM32 Independent WatchDoG (IWDG)
> +---------------------------------
> +
> +Required properties:
> +- compatible: "st,stm32-iwdg"
> +- reg: physical base address and length of the registers set for the device
> +- clocks: must contain a single entry describing the clock input
> +
> +Example:
> +
> +iwdg@40003000 {
> +	compatible = "st,stm32-iwdg";
> +	reg = <0x40003000 0x400>;
> +	clocks = <&clk_lsi>;
> +};
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 2/5] drivers: watchdog: Add STM32 IWDG driver
From: Guenter Roeck @ 2017-03-29  3:15 UTC (permalink / raw)
  To: Yannick Fertre, Wim Van Sebroeck, Rob Herring, Alexandre TORGUE,
	Benjamin Gaignard, Maxime Coquelin
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Philippe Cornu,
	Gabriel FERNANDEZ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490195083-25117-3-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>

On 03/22/2017 08:04 AM, Yannick Fertre wrote:
> This patch adds IWDG (Independent WatchDoG) support for STM32 platform.
>
> Change-Id: Iab218745fc25566f12558fae7f52e2f8c21db74e

No gerrit tags, please.

> Signed-off-by: Yannick FERTRE <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/watchdog/Kconfig      |  11 ++
>  drivers/watchdog/Makefile     |   1 +
>  drivers/watchdog/stm32_iwdg.c | 289 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 301 insertions(+)
>  create mode 100644 drivers/watchdog/stm32_iwdg.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 52a70ee..d9745a5 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called zx2967_wdt.
>
> +config STM32_WATCHDOG
> +	tristate "STM32 Independent WatchDoG (IWDG) support"
> +	depends on ARCH_STM32
> +	select WATCHDOG_CORE
> +	default y
> +	help
> +	  Say Y here to include Watchdog timer support.

... for <pick your text>.

> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called stm32_iwdg.
> +
>  # AVR32 Architecture
>
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a2126e2..a35e423 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
>  obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
>  obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
>  obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> +obj-$(CONFIG_STM32_WATCHDOG) += stm32_iwdg.o
>
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> new file mode 100644
> index 0000000..507dfc4
> --- /dev/null
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -0,0 +1,289 @@
> +/*
> + * Driver for STM32 Independent Watchdog
> + *
> + * Copyright (C) Yannick Fertre 2017
> + * Author: Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> + *
> + * This driver is based on tegra_wdt.c
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +/* minimum watchdog trigger timeout, in seconds */
> +#define MIN_WDT_TIMEOUT	1
> +
> +/* IWDG registers */
> +#define IWDG_KR		0x00 /* Key register */
> +#define IWDG_PR		0x04 /* Prescaler Register */
> +#define IWDG_RLR	0x08 /* ReLoad Register */
> +#define IWDG_SR		0x0C /* Status Register */
> +#define IWDG_WINR	0x10 /* Windows Register */
> +
> +/* IWDG_KR register bit mask */
> +#define KR_KEY_RELOAD	0xAAAA /* reload counter enable */
> +#define KR_KEY_ENABLE	0xCCCC /* peripheral enable */
> +#define KR_KEY_EWA	0x5555 /* write access enable */
> +#define KR_KEY_DWA	0x0000 /* write access disable */
> +
> +/* IWDG_PR register bit values */
> +#define PR_4    0x00 /* prescaler set to 4 */
> +#define PR_8    0x01 /* prescaler set to 8 */
> +#define PR_16   0x02 /* prescaler set to 16 */
> +#define PR_32   0x03 /* prescaler set to 32 */
> +#define PR_64   0x04 /* prescaler set to 64 */
> +#define PR_128  0x05 /* prescaler set to 128 */
> +#define PR_256	0x06 /* prescaler set to 256 */
> +
> +/* IWDG_RLR register values */
> +#define RLR_MAX		0xFFF /* max value supported by reload register */
> +
> +/* IWDG_SR register bit mask */
> +#define FLAG_PVU	BIT(0) /* Watchdog prescaler value update */
> +#define FLAG_RVU	BIT(1) /* Watchdog counter reload value update */
> +
> +/* set timeout to 100000 us */
> +#define TIMEOUT_US	100000
> +#define SLEEP_US	1000
> +
> +struct stm32_iwdg {
> +	struct watchdog_device	wdd;
> +	struct device		*dev;
> +	void __iomem		*regs;
> +	struct clk		*clk;
> +	unsigned int		rate;
> +};
> +
> +static int heartbeat = MIN_WDT_TIMEOUT;
> +module_param(heartbeat, int, 0x0);
> +MODULE_PARM_DESC(heartbeat,
> +		 "Watchdog heartbeats in seconds. (default = "
> +		 __MODULE_STRING(WDT_HEARTBEAT) ")");
> +
> +static inline u32 reg_read(void __iomem *base, u32 reg)
> +{
> +	return readl_relaxed(base + reg);
> +}
> +
> +static inline void reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> +	writel_relaxed(val, base + reg);
> +}
> +
> +static int stm32_iwdg_start(struct watchdog_device *wdd)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +	u32 val = FLAG_PVU | FLAG_RVU;
> +	u32 reload;
> +	int ret;
> +
> +	dev_dbg(wdt->dev, "%s\n", __func__);
> +
> +	/* prescaler fixed to 256 */
> +	reload = (wdd->timeout * wdt->rate) / 256;
> +	if (reload > RLR_MAX + 1) {
> +		dev_err(wdt->dev,
> +			"Watchdog doesn't support reload value: %d\n", reload);
> +		return -EINVAL;
> +	}

This should not happen if min_timeout and max_timeout are set properly.

> +
> +	/* enable watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
> +	/* set prescaler & reload registers */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA);
> +	reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */
> +	reg_write(wdt->regs, IWDG_RLR, reload - 1);
> +
> +	/* wait for the registers to be updated (max 100ms) */
> +	ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val,
> +					 !(val & (FLAG_PVU | FLAG_RVU)),
> +					 SLEEP_US, TIMEOUT_US);
> +	if (ret) {
> +		dev_err(wdt->dev,
> +			"Fail to set prescaler or reload registers\n");
> +		return -EINVAL;

Any reason for overriding the return value ?

> +	}
> +
> +	/* reload watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> +	return 0;
> +}
> +
> +static int stm32_iwdg_stop(struct watchdog_device *wdd)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +
> +	if (watchdog_active(wdd)) {
> +		dev_err(wdt->dev,
> +			"Watchdog can't be stopped once started(no way out)\n");
> +		return -EPERM;

The infrastructure takes care of this. Don't provide a stop function, and provide
the maximum hardware timeout.

> +	}
> +
> +	return 0;
> +}
> +
> +static int stm32_iwdg_ping(struct watchdog_device *wdd)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +
> +	dev_dbg(wdt->dev, "%s\n", __func__);
> +
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> +	return 0;
> +}
> +
> +static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
> +				  unsigned int timeout)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +
> +	dev_dbg(wdt->dev, "%s timeout: %d sec\n", __func__, timeout);
> +
> +	wdd->timeout = timeout;
> +
> +	if (watchdog_active(wdd))
> +		return stm32_iwdg_start(wdd);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info stm32_iwdg_info = {
> +	.options	= WDIOF_SETTIMEOUT |
> +			  WDIOF_MAGICCLOSE |
> +			  WDIOF_KEEPALIVEPING,
> +	.identity	= "STM32 Independent Watchdog",
> +};
> +
> +static struct watchdog_ops stm32_iwdg_ops = {
> +	.owner		= THIS_MODULE,
> +	.start		= stm32_iwdg_start,
> +	.stop		= stm32_iwdg_stop,
> +	.ping		= stm32_iwdg_ping,
> +	.set_timeout	= stm32_iwdg_set_timeout,
> +};
> +
> +static int stm32_iwdg_probe(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdd;
> +	struct stm32_iwdg *wdt;
> +	struct resource *res;
> +	void __iomem *regs;
> +	struct clk *clk;
> +	int max_wdt_timeout;
> +	int ret;
> +
> +	/* This is the timer base. */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(regs)) {
> +		dev_err(&pdev->dev, "Could not get resource\n");
> +		return PTR_ERR(regs);
> +	}
> +
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk)) {
> +		dev_err(&pdev->dev, "Unable to get clock\n");
> +		return PTR_ERR(clk);
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Allocate our watchdog driver data, which has the
> +	 * struct watchdog_device nested within it.
> +	 */
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	/* Initialize struct stm32_iwdg. */
> +	wdt->regs = regs;
> +	wdt->dev = &pdev->dev;
> +	wdt->clk = clk;
> +	/*
> +	 * iwdg is clocked by its own dedicated low-speed clock (LSI)
> +	 * at 32khz.
> +	 */
> +	wdt->rate = 32 * 1024;

Why not clk_get_rate() ?

> +
> +	/* get max timeout & set heartbeat */
> +	max_wdt_timeout = ((RLR_MAX + 1) * 256) / wdt->rate;
> +	heartbeat = max_wdt_timeout;

What is the value of overwriting this variable ? Why don't you just use max_wdt_timeout,
and what is the point of making heartbeat configurable if you overwrite it anyway ?

I would suggest to use watchdog_init_timeout() to set optional non-default
timeout values, especially since this also enables getting the timeout value from
devicetree.

> +
> +	/* Initialize struct watchdog_device. */
> +	wdd = &wdt->wdd;
> +	wdd->timeout = heartbeat;
> +	wdd->info = &stm32_iwdg_info;
> +	wdd->ops = &stm32_iwdg_ops;
> +	wdd->min_timeout = MIN_WDT_TIMEOUT;
> +	wdd->max_timeout = max_wdt_timeout;

You might want to consider setting max_hw_heartbeat_ms instead and drop the stop function.

> +	watchdog_set_drvdata(wdd, wdt);
> +	watchdog_set_nowayout(wdd, true);
> +
> +	ret = watchdog_register_device(wdd);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"failed to register watchdog device\n");
> +		goto err;
> +	}
> +
> +	platform_set_drvdata(pdev, wdt);
> +
> +	dev_info(&pdev->dev,
> +		 "initialized (heartbeat = %d sec)\n", heartbeat);
> +	return 0;
> +
> +err:
> +	clk_disable_unprepare(clk);
> +	return ret;
> +}
> +
> +static int stm32_iwdg_remove(struct platform_device *pdev)
> +{
> +	struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(&wdt->wdd);
> +	clk_disable_unprepare(wdt->clk);
> +
> +	dev_info(&pdev->dev, "removed watchdog device\n");

Is this message really useful ? I think it is just noise.

> +	return 0;
> +}
> +
> +static const struct of_device_id stm32_iwdg_of_match[] = {
> +	{ .compatible = "st,stm32-iwdg" },
> +	{ /* end node */ }
> +};
> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
> +
> +static struct platform_driver stm32_iwdg_driver = {
> +	.probe		= stm32_iwdg_probe,
> +	.remove		= stm32_iwdg_remove,
> +	.driver = {
> +		.name	= "iwdg",
> +		.of_match_table = stm32_iwdg_of_match,
> +	},
> +};
> +module_platform_driver(stm32_iwdg_driver);
> +
> +MODULE_AUTHOR("Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 Independent Watchdog Driver");
> +MODULE_LICENSE("GPL v2");
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Leo Yan @ 2017-03-29  3:07 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
	John Stultz, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
	sudeep.holla
In-Reply-To: <31be033f-514e-e48a-3ba2-a5c5cd477548@arm.com>

Hi Suzuki,

On Mon, Mar 27, 2017 at 05:34:57PM +0100, Suzuki K Poulose wrote:
> On 25/03/17 18:23, Leo Yan wrote:

[...]

> Leo,
> 
> Thanks a lot for the quick rework. I don't fully understand (yet!) why we need the
> idle_constraint. I will leave it for Sudeep to comment on it, as he is the expert
> in that area. Some minor comments below.

Thanks a lot for quick reviewing :)

> >Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >---
> > drivers/hwtracing/coresight/Kconfig               |  11 +
> > drivers/hwtracing/coresight/Makefile              |   1 +
> > drivers/hwtracing/coresight/coresight-cpu-debug.c | 704 ++++++++++++++++++++++
> > 3 files changed, 716 insertions(+)
> > create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
> >
> >diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> >index 130cb21..18d7931 100644
> >--- a/drivers/hwtracing/coresight/Kconfig
> >+++ b/drivers/hwtracing/coresight/Kconfig
> >@@ -89,4 +89,15 @@ config CORESIGHT_STM
> > 	  logging useful software events or data coming from various entities
> > 	  in the system, possibly running different OSs
> >
> >+config CORESIGHT_CPU_DEBUG
> >+	tristate "CoreSight CPU Debug driver"
> >+	depends on ARM || ARM64
> >+	depends on DEBUG_FS
> >+	help
> >+	  This driver provides support for coresight debugging module. This
> >+	  is primarily used to dump sample-based profiling registers when
> >+	  system triggers panic, the driver will parse context registers so
> >+	  can quickly get to know program counter (PC), secure state,
> >+	  exception level, etc.
> 
> May be we should mention/warn the user about the possible caveats of using
> this feature to help him make a better decision ? And / Or we should add a documentation
> for it. We have collected some real good information over the discussions and
> it is a good idea to capture it somewhere.

Sure, I will add a documentation for this.

[...]

> >+static struct pm_qos_request debug_qos_req;
> >+static int idle_constraint = PM_QOS_DEFAULT_VALUE;
> >+module_param(idle_constraint, int, 0600);
> >+MODULE_PARM_DESC(idle_constraint, "Latency requirement in microseconds for CPU "
> >+		 "idle states (default is -1, which means have no limiation "
> >+		 "to CPU idle states; 0 means disabling all idle states; user "
> >+		 "can choose other platform dependent values so can disable "
> >+		 "specific idle states for the platform)");
> 
> Correct me if I am wrong,
> 
> All we want to do is disable the CPUIdle explicitly if the user knows that this
> could be a problem to use CPU debug on his platform. So, in effect, we should
> only be using idle_constraint = 0 or -1.
> 
> In which case, we could make it easier for the user to tell us, either
> 
>  0 - Don't do anything with CPUIdle (default)
>  1 - Disable CPUIdle for me as I know the platform has issues with CPU debug and CPUidle.

The reason for not using bool flag is: usually SoC may have many idle
states, so if user wants to partially enable some states then can set
the latency to constraint.

But of course, we can change this to binary value as you suggested,
this means turn on of turn off all states. The only one reason to use
latency value is it is more friendly for hardware design, e.g. some
platforms can enable partial states to save power and avoid overheat
after using this driver.

If you guys think this is a bit over design, I will follow up your
suggestion. I also have some replying in Mathieu's reviewing, please
help review as well.

> than explaining the miscrosecond latency etc and make the appropriate calls underneath.
> something like (not necessarily the same name) :
> 
> module_param(broken_with_cpuidle, bool, 0600);
> MODULE_PARAM_DESC(broken_with_cpuidle, "Specifies whether the CPU debug has issues with CPUIdle on"
> 				       " the platform. Non-zero value implies CPUIdle has to be"
> 				       " explicitly disabled.",);

[...]

> >+	/*
> >+	 * Unfortunately the CPU cannot be powered up, so return
> >+	 * back and later has no permission to access other
> >+	 * registers. For this case, should set 'idle_constraint'
> >+	 * to ensure CPU power domain is enabled!
> >+	 */
> >+	if (!(drvdata->edprsr & EDPRSR_PU)) {
> >+		pr_err("%s: power up request for CPU%d failed\n",
> >+			__func__, drvdata->cpu);
> >+		goto out;
> >+	}
> >+
> >+out_powered_up:
> >+	debug_os_unlock(drvdata);
> 
> Question: Do we need a matching debug_os_lock() once we are done ?

I have checked ARM ARMv8, but there have no detailed description for
this. I refered coresight-etmv4 code and Mike's pseudo code, ther have
no debug_os_lock() related operations.

Mike, Mathieu, could you also help confirm this?

[...]

> >+static void debug_init_arch_data(void *info)
> >+{
> >+	struct debug_drvdata *drvdata = info;
> >+	u32 mode, pcsr_offset;
> >+
> >+	CS_UNLOCK(drvdata->base);
> >+
> >+	debug_os_unlock(drvdata);
> >+
> >+	/* Read device info */
> >+	drvdata->eddevid  = readl_relaxed(drvdata->base + EDDEVID);
> >+	drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
> 
> As mentioned above, both of these registers are only need at init time to
> figure out the flags we set here. So we could remove them.
> 
> >+
> >+	CS_LOCK(drvdata->base);
> >+
> >+	/* Parse implementation feature */
> >+	mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
> >+	pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
> 
> 
> >+
> >+	if (mode == EDDEVID_IMPL_NONE) {
> >+		drvdata->edpcsr_present  = false;
> >+		drvdata->edcidsr_present = false;
> >+		drvdata->edvidsr_present = false;
> >+	} else if (mode == EDDEVID_IMPL_EDPCSR) {
> >+		drvdata->edpcsr_present  = true;
> >+		drvdata->edcidsr_present = false;
> >+		drvdata->edvidsr_present = false;
> >+	} else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
> >+		if (!IS_ENABLED(CONFIG_64BIT) &&
> >+			(pcsr_offset == EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32))
> >+			drvdata->edpcsr_present = false;
> >+		else
> >+			drvdata->edpcsr_present = true;
> 
> Sorry, I forgot why we do this check only in this mode. Shouldn't this be
> common to all modes (of course which implies PCSR is present) ?

No. PCSROffset is defined differently in ARMv7 and ARMv8; So finally we
simplize PCSROffset value :
0000 - Sample offset applies based on the instruction state (indicated by PCSR[0])
0001 - No offset applies.
0010 - No offset applies, but do not use in AArch32 mode!

So we need handle the corner case is when CPU runs AArch32 mode and
PCSRoffset = 'b0010. Other cases the pcsr should be present.

[...]

Other suggestions are good for me, will take them in next version.

Thanks,
Leo Yan

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: ls1012a: add crypto node
From: Shawn Guo @ 2017-03-29  3:04 UTC (permalink / raw)
  To: Horia Geantă
  Cc: Rob Herring, Mark Rutland, devicetree, Herbert Xu, Harninder Rai,
	Catalin Marinas, Bhaskar Upadhaya, Will Deacon, Dan Douglass,
	linux-crypto, David S. Miller, linux-arm-kernel
In-Reply-To: <20170328114619.16568-1-horia.geanta@nxp.com>

On Tue, Mar 28, 2017 at 02:46:19PM +0300, Horia Geantă wrote:
> LS1012A has a SEC v5.4 security engine.
> 
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v3 0/5] STM32 Independant watchdog
From: Guenter Roeck @ 2017-03-29  3:00 UTC (permalink / raw)
  To: Yannick Fertre, Wim Van Sebroeck, Rob Herring, Alexandre TORGUE,
	Benjamin Gaignard, Maxime Coquelin
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Philippe Cornu,
	Gabriel FERNANDEZ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490195083-25117-1-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>

Subject: s/Independant/Independent/

On 03/22/2017 08:04 AM, Yannick Fertre wrote:
> Version 3:
> - Update typo into  bindings file
> - Reorder node in devicetree (ordering by address)
> - Remove unecessary lines in commits
>
> Version 2:
> - Add commit messages
>
> Version 1:
> - Initial commit
>
> The purpose of this set of patches is to add a new watchdog driver for
> stm32f429.
> This driver was developed and tested on evaluation board stm32429i.
>
> Yannick Fertre (5):
>   dt-bindings: Document STM32 IWDG bindings
>   drivers: watchdog: Add STM32 IWDG driver
>   ARM: dts: stm32: Add watchdog support for STM32F429 SoC
>   ARM: dts: stm32: Add watchdog support for STM32F429 eval board
>   ARM: configs: Add watchdog support in STM32 defconfig
>
>  .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |  15 ++
>  arch/arm/boot/dts/stm32429i-eval.dts               |   4 +
>  arch/arm/boot/dts/stm32f429.dtsi                   |   9 +-
>  arch/arm/configs/stm32_defconfig                   |   1 +
>  drivers/watchdog/Kconfig                           |  11 +
>  drivers/watchdog/Makefile                          |   1 +
>  drivers/watchdog/stm32_iwdg.c                      | 289 +++++++++++++++++++++
>  7 files changed, 329 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>  create mode 100644 drivers/watchdog/stm32_iwdg.c
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] auxdisplay: hd44780: Fix DT properties to include units of measurement
From: Rob Herring @ 2017-03-29  2:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Willy Tarreau, Miguel Ojeda Sandonis,
	Mark Rutland, Ksenija Stanojevic, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490275537-3769-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

On Thu, Mar 23, 2017 at 02:25:37PM +0100, Geert Uytterhoeven wrote:
> DT properties specifying physical properties should contain appropriate
> suffices indicating the units of measurement.
> 
> Hence amend the HD44780 DT bindings to add "chars" suffixes to the
> "display-height" and "display-width" properties, and update the driver
> to parse them.
> 
> Fixes: dd9502a9e9156dd8 ("dt-bindings: auxdisplay: Add bindings for Hitachi HD44780")
> Fixes: d47d88361feea2ce ("auxdisplay: Add HD44780 Character LCD support")
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> ---
> Against char-misc-next
> 
>  Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt | 11 ++++++-----
>  drivers/auxdisplay/hd44780.c                                 |  5 +++--
>  2 files changed, 9 insertions(+), 7 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 1/2] Documentation: dt-bindings: Document bindings for Aspeed ADC
From: Joel Stanley @ 2017-03-29  2:33 UTC (permalink / raw)
  To: Rick Altherr
  Cc: OpenBMC Maillist, Linux Kernel Mailing List,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Hartmut Knaack, Rob Herring,
	Lars-Peter Clausen, Mark Rutland, Jonathan Cameron,
	Peter Meerwald-Stadler
In-Reply-To: <20170328215259.31622-1-raltherr-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Wed, Mar 29, 2017 at 8:22 AM, Rick Altherr <raltherr-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Signed-off-by: Rick Altherr <raltherr-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---

Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

>
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
> - Consistently write hex contstants with lowercase letters
> - Drop model numbers from description as same IP is used in every generation
>
> Changes in v2:
> - Rewritten as an IIO ADC device
>
>  .../devicetree/bindings/iio/adc/aspeed_adc.txt       | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
> new file mode 100644
> index 000000000000..674e133b7cd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
> @@ -0,0 +1,20 @@
> +Aspeed ADC
> +
> +This device is a 10-bit converter for 16 voltage channels.  All inputs are
> +single ended.
> +
> +Required properties:
> +- compatible: Should be "aspeed,ast2400-adc" or "aspeed,ast2500-adc"
> +- reg: memory window mapping address and length
> +- clocks: Input clock used to derive the sample clock. Expected to be the
> +          SoC's APB clock.
> +- #io-channel-cells: Must be set to <1> to indicate channels are selected
> +                     by index.
> +
> +Example:
> +       adc@1e6e9000 {
> +               compatible = "aspeed,ast2400-adc";
> +               reg = <0x1e6e9000 0xb0>;
> +               clocks = <&clk_apb>;
> +               #io-channel-cells = <1>;
> +       };
> --
> 2.12.2.564.g063fe858b8-goog
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: Linus Walleij @ 2017-03-29  2:30 UTC (permalink / raw)
  To: jacopo
  Cc: Geert Uytterhoeven, Jacopo Mondi, Geert Uytterhoeven,
	Laurent Pinchart, Chris Brandt, Rob Herring, Mark Rutland,
	Russell King, Linux-Renesas, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <b9eaac8a63dced229bb152ba412e8ec1@jmondi.org>

On Tue, Mar 28, 2017 at 4:38 PM,  <jacopo@jmondi.org> wrote:

>> The fact that historically all the early adopters of pinctrl in device
>> tree
>> have these funky custom bindings is unfortunate but just something
>> that we need to live with.
>>
>
> To avoid any confusion, please bear with me and clarify this once and for
> all,
> since I'm not certain I fully got you here.
>
> Are you suggesting:
>
> 1) Use "pins" property with the currently implemented ABI (which slightly
> differs
>    from the standard documented one as explained above. Not sure it is fine
> overriding
>    it or not)

Correction: you should be using the property "pinmux", because you
are setting group and function at the same time.

See for example:
include/dt-bindings/pinctrl/mt65xx.h

And how that is used in:
arch/arm/boot/dts/mt2701-pinfunc.h
arch/arm/boot/dts/mt2701-evb.dts

The docs are here:
Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt

I'm sorry that "pinmux" is not part of the generic documentation, it'd be
great if you would like to add it with a patch.

Yours,
Linus Walleij

^ permalink raw reply


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