All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6] FlexRM support in VFIO platform
@ 2017-08-29  4:04 ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-08-29  4:04 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson
  Cc: Scott Branden, linux-kernel, linux-arm-kernel, iommu, kvm,
	bcm-kernel-feedback-list, Anup Patel

This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver.

The patches are based on Linux-4.13-rc3 and can also be
found at flexrm-vfio-v6 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v5:
 - Make kconfig option VFIO_PLATFORM_BCMFLEXRM_RESET
   default to ARCH_BCM_IPROC

Changes since v4:
 - Use "--timeout" instead of "timeout--" in
   vfio_platform_bcmflexrm_shutdown()

Changes since v3:
 - Improve "depends on" for Kconfig option
   VFIO_PLATFORM_BCMFLEXRM_RESET
 - Fix typo in pr_warn() called by
   vfio_platform_bcmflexrm_shutdown()
 - Return error from vfio_platform_bcmflexrm_shutdown()
   when FlexRM ring flush timeout happens

Changes since v2:
 - Remove PATCH1 because fixing VFIO no-IOMMU mode is
   a separate topic

Changes since v1:
 - Remove iommu_present() check in vfio_iommu_group_get()
 - Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
   required
 - Move additional comments out of license header in
   vfio_platform_bcmflexrm.c

Anup Patel (1):
  vfio: platform: reset: Add Broadcom FlexRM reset module

 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] FlexRM support in VFIO platform
@ 2017-08-29  4:04 ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel via iommu @ 2017-08-29  4:04 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA, Scott Branden,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver.

The patches are based on Linux-4.13-rc3 and can also be
found at flexrm-vfio-v6 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v5:
 - Make kconfig option VFIO_PLATFORM_BCMFLEXRM_RESET
   default to ARCH_BCM_IPROC

Changes since v4:
 - Use "--timeout" instead of "timeout--" in
   vfio_platform_bcmflexrm_shutdown()

Changes since v3:
 - Improve "depends on" for Kconfig option
   VFIO_PLATFORM_BCMFLEXRM_RESET
 - Fix typo in pr_warn() called by
   vfio_platform_bcmflexrm_shutdown()
 - Return error from vfio_platform_bcmflexrm_shutdown()
   when FlexRM ring flush timeout happens

Changes since v2:
 - Remove PATCH1 because fixing VFIO no-IOMMU mode is
   a separate topic

Changes since v1:
 - Remove iommu_present() check in vfio_iommu_group_get()
 - Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
   required
 - Move additional comments out of license header in
   vfio_platform_bcmflexrm.c

Anup Patel (1):
  vfio: platform: reset: Add Broadcom FlexRM reset module

 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] FlexRM support in VFIO platform
@ 2017-08-29  4:04 ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-08-29  4:04 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver.

The patches are based on Linux-4.13-rc3 and can also be
found at flexrm-vfio-v6 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v5:
 - Make kconfig option VFIO_PLATFORM_BCMFLEXRM_RESET
   default to ARCH_BCM_IPROC

Changes since v4:
 - Use "--timeout" instead of "timeout--" in
   vfio_platform_bcmflexrm_shutdown()

Changes since v3:
 - Improve "depends on" for Kconfig option
   VFIO_PLATFORM_BCMFLEXRM_RESET
 - Fix typo in pr_warn() called by
   vfio_platform_bcmflexrm_shutdown()
 - Return error from vfio_platform_bcmflexrm_shutdown()
   when FlexRM ring flush timeout happens

Changes since v2:
 - Remove PATCH1 because fixing VFIO no-IOMMU mode is
   a separate topic

Changes since v1:
 - Remove iommu_present() check in vfio_iommu_group_get()
 - Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
   required
 - Move additional comments out of license header in
   vfio_platform_bcmflexrm.c

Anup Patel (1):
  vfio: platform: reset: Add Broadcom FlexRM reset module

 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-08-29  4:04   ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-08-29  4:04 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson
  Cc: Scott Branden, linux-kernel, linux-arm-kernel, iommu, kvm,
	bcm-kernel-feedback-list, Anup Patel

This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Oza Oza <oza.oza@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
index 70cccc5..392e3c0 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
 	  Enables the VFIO platform driver to handle reset for AMD XGBE
 
 	  If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+	tristate "VFIO support for Broadcom FlexRM reset"
+	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+	default ARCH_BCM_IPROC
+	help
+	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+	  If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
 
 obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
 obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 0000000..966a813
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,100 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE					0x10000
+#define RING_VER_MAGIC					0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER					0x000
+#define RING_CONTROL					0x034
+#define RING_FLUSH_DONE					0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT				5
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK					0x1
+
+static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+	unsigned int timeout;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring + RING_CONTROL);
+
+	/* Flush ring with timeout of 1s */
+	timeout = 1000;
+	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+	do {
+		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
+			break;
+		mdelay(1);
+	} while (--timeout);
+
+	if (!timeout) {
+		pr_warn("VFIO FlexRM shutdown timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+	int rc = 0;
+	void __iomem *ring;
+	struct vfio_platform_region *reg = &vdev->regions[0];
+
+	/* Map FlexRM ring registers if not mapped */
+	if (!reg->ioaddr) {
+		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+		if (!reg->ioaddr)
+			return -ENOMEM;
+	}
+
+	/* Discover and shutdown each FlexRM ring */
+	for (ring = reg->ioaddr;
+	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
+		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
+			rc = vfio_platform_bcmflexrm_shutdown(ring);
+			if (rc)
+				break;
+		}
+	}
+
+	return rc;
+}
+
+module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
+			  vfio_platform_bcmflexrm_reset);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
+MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-08-29  4:04   ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel via iommu @ 2017-08-29  4:04 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA, Scott Branden,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
index 70cccc5..392e3c0 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
 	  Enables the VFIO platform driver to handle reset for AMD XGBE
 
 	  If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+	tristate "VFIO support for Broadcom FlexRM reset"
+	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+	default ARCH_BCM_IPROC
+	help
+	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+	  If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
 
 obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
 obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 0000000..966a813
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,100 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE					0x10000
+#define RING_VER_MAGIC					0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER					0x000
+#define RING_CONTROL					0x034
+#define RING_FLUSH_DONE					0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT				5
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK					0x1
+
+static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+	unsigned int timeout;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring + RING_CONTROL);
+
+	/* Flush ring with timeout of 1s */
+	timeout = 1000;
+	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+	do {
+		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
+			break;
+		mdelay(1);
+	} while (--timeout);
+
+	if (!timeout) {
+		pr_warn("VFIO FlexRM shutdown timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+	int rc = 0;
+	void __iomem *ring;
+	struct vfio_platform_region *reg = &vdev->regions[0];
+
+	/* Map FlexRM ring registers if not mapped */
+	if (!reg->ioaddr) {
+		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+		if (!reg->ioaddr)
+			return -ENOMEM;
+	}
+
+	/* Discover and shutdown each FlexRM ring */
+	for (ring = reg->ioaddr;
+	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
+		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
+			rc = vfio_platform_bcmflexrm_shutdown(ring);
+			if (rc)
+				break;
+		}
+	}
+
+	return rc;
+}
+
+module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
+			  vfio_platform_bcmflexrm_reset);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>");
+MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-08-29  4:04   ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-08-29  4:04 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Oza Oza <oza.oza@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/vfio/platform/reset/Kconfig                |   9 ++
 drivers/vfio/platform/reset/Makefile               |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
index 70cccc5..392e3c0 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
 	  Enables the VFIO platform driver to handle reset for AMD XGBE
 
 	  If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+	tristate "VFIO support for Broadcom FlexRM reset"
+	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+	default ARCH_BCM_IPROC
+	help
+	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+	  If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
 
 obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
 obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 0000000..966a813
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,100 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE					0x10000
+#define RING_VER_MAGIC					0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER					0x000
+#define RING_CONTROL					0x034
+#define RING_FLUSH_DONE					0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT				5
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK					0x1
+
+static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+	unsigned int timeout;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring + RING_CONTROL);
+
+	/* Flush ring with timeout of 1s */
+	timeout = 1000;
+	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+	do {
+		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
+			break;
+		mdelay(1);
+	} while (--timeout);
+
+	if (!timeout) {
+		pr_warn("VFIO FlexRM shutdown timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+	int rc = 0;
+	void __iomem *ring;
+	struct vfio_platform_region *reg = &vdev->regions[0];
+
+	/* Map FlexRM ring registers if not mapped */
+	if (!reg->ioaddr) {
+		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+		if (!reg->ioaddr)
+			return -ENOMEM;
+	}
+
+	/* Discover and shutdown each FlexRM ring */
+	for (ring = reg->ioaddr;
+	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
+		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
+			rc = vfio_platform_bcmflexrm_shutdown(ring);
+			if (rc)
+				break;
+		}
+	}
+
+	return rc;
+}
+
+module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
+			  vfio_platform_bcmflexrm_reset);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
+MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
  2017-08-29  4:04   ` Anup Patel via iommu
  (?)
@ 2017-08-29 14:09     ` Konrad Rzeszutek Wilk
  -1 siblings, 0 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 14:09 UTC (permalink / raw)
  To: Anup Patel
  Cc: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson, Scott Branden, linux-kernel, linux-arm-kernel,
	iommu, kvm, bcm-kernel-feedback-list

On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
> This patch adds Broadcom FlexRM low-level reset for
> VFIO platform.
> 

Is there an document that explains and /or details the various
registers?
> It will do the following:
> 1. Disable/Deactivate each FlexRM ring
> 2. Flush each FlexRM ring
> 
> The cleanup sequence for FlexRM rings is adapted from
> Broadcom FlexRM mailbox driver.
> 
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>  drivers/vfio/platform/reset/Makefile               |   1 +
>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> 
> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> index 70cccc5..392e3c0 100644
> --- a/drivers/vfio/platform/reset/Kconfig
> +++ b/drivers/vfio/platform/reset/Kconfig
> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>  	  Enables the VFIO platform driver to handle reset for AMD XGBE
>  
>  	  If you don't know what to do here, say N.
> +
> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> +	tristate "VFIO support for Broadcom FlexRM reset"
> +	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> +
> +	  If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
> index 93f4e23..8d9874b 100644
> --- a/drivers/vfio/platform/reset/Makefile
> +++ b/drivers/vfio/platform/reset/Makefile
> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>  
>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> new file mode 100644
> index 0000000..966a813
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> @@ -0,0 +1,100 @@
> +/*
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * This driver provides reset support for Broadcom FlexRM ring manager
> + * to VFIO platform.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include "vfio_platform_private.h"
> +
> +/* FlexRM configuration */
> +#define RING_REGS_SIZE					0x10000
> +#define RING_VER_MAGIC					0x76303031
> +
> +/* Per-Ring register offsets */
> +#define RING_VER					0x000
> +#define RING_CONTROL					0x034
> +#define RING_FLUSH_DONE					0x038
> +
> +/* Register RING_CONTROL fields */
> +#define CONTROL_FLUSH_SHIFT				5
> +
> +/* Register RING_FLUSH_DONE fields */
> +#define FLUSH_DONE_MASK					0x1
> +
> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> +{
> +	unsigned int timeout;
> +
> +	/* Disable/inactivate ring */
> +	writel_relaxed(0x0, ring + RING_CONTROL);
> +
> +	/* Flush ring with timeout of 1s */
> +	timeout = 1000;

Perhaps a #define for this value?

> +	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> +	do {
> +		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
> +			break;
> +		mdelay(1);
> +	} while (--timeout);
> +
> +	if (!timeout) {
> +		pr_warn("VFIO FlexRM shutdown timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> +{
> +	int rc = 0;
> +	void __iomem *ring;
> +	struct vfio_platform_region *reg = &vdev->regions[0];
> +
> +	/* Map FlexRM ring registers if not mapped */
> +	if (!reg->ioaddr) {
> +		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
> +		if (!reg->ioaddr)
> +			return -ENOMEM;
> +	}
> +
> +	/* Discover and shutdown each FlexRM ring */
> +	for (ring = reg->ioaddr;
> +	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
> +		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
> +			rc = vfio_platform_bcmflexrm_shutdown(ring);
> +			if (rc)
> +				break;

You sure? Don't you want to continue trying to reset all of them and
then return rc at the end?

> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
> +			  vfio_platform_bcmflexrm_reset);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
> +MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-08-29 14:09     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 14:09 UTC (permalink / raw)
  To: Anup Patel
  Cc: Baptiste Reynal, kvm, Scott Branden, Joerg Roedel, Will Deacon,
	linux-kernel, iommu, Alex Williamson, bcm-kernel-feedback-list,
	Robin Murphy, linux-arm-kernel

On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
> This patch adds Broadcom FlexRM low-level reset for
> VFIO platform.
> 

Is there an document that explains and /or details the various
registers?
> It will do the following:
> 1. Disable/Deactivate each FlexRM ring
> 2. Flush each FlexRM ring
> 
> The cleanup sequence for FlexRM rings is adapted from
> Broadcom FlexRM mailbox driver.
> 
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>  drivers/vfio/platform/reset/Makefile               |   1 +
>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> 
> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> index 70cccc5..392e3c0 100644
> --- a/drivers/vfio/platform/reset/Kconfig
> +++ b/drivers/vfio/platform/reset/Kconfig
> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>  	  Enables the VFIO platform driver to handle reset for AMD XGBE
>  
>  	  If you don't know what to do here, say N.
> +
> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> +	tristate "VFIO support for Broadcom FlexRM reset"
> +	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> +
> +	  If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
> index 93f4e23..8d9874b 100644
> --- a/drivers/vfio/platform/reset/Makefile
> +++ b/drivers/vfio/platform/reset/Makefile
> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>  
>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> new file mode 100644
> index 0000000..966a813
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> @@ -0,0 +1,100 @@
> +/*
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * This driver provides reset support for Broadcom FlexRM ring manager
> + * to VFIO platform.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include "vfio_platform_private.h"
> +
> +/* FlexRM configuration */
> +#define RING_REGS_SIZE					0x10000
> +#define RING_VER_MAGIC					0x76303031
> +
> +/* Per-Ring register offsets */
> +#define RING_VER					0x000
> +#define RING_CONTROL					0x034
> +#define RING_FLUSH_DONE					0x038
> +
> +/* Register RING_CONTROL fields */
> +#define CONTROL_FLUSH_SHIFT				5
> +
> +/* Register RING_FLUSH_DONE fields */
> +#define FLUSH_DONE_MASK					0x1
> +
> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> +{
> +	unsigned int timeout;
> +
> +	/* Disable/inactivate ring */
> +	writel_relaxed(0x0, ring + RING_CONTROL);
> +
> +	/* Flush ring with timeout of 1s */
> +	timeout = 1000;

Perhaps a #define for this value?

> +	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> +	do {
> +		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
> +			break;
> +		mdelay(1);
> +	} while (--timeout);
> +
> +	if (!timeout) {
> +		pr_warn("VFIO FlexRM shutdown timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> +{
> +	int rc = 0;
> +	void __iomem *ring;
> +	struct vfio_platform_region *reg = &vdev->regions[0];
> +
> +	/* Map FlexRM ring registers if not mapped */
> +	if (!reg->ioaddr) {
> +		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
> +		if (!reg->ioaddr)
> +			return -ENOMEM;
> +	}
> +
> +	/* Discover and shutdown each FlexRM ring */
> +	for (ring = reg->ioaddr;
> +	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
> +		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
> +			rc = vfio_platform_bcmflexrm_shutdown(ring);
> +			if (rc)
> +				break;

You sure? Don't you want to continue trying to reset all of them and
then return rc at the end?

> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
> +			  vfio_platform_bcmflexrm_reset);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
> +MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-08-29 14:09     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
> This patch adds Broadcom FlexRM low-level reset for
> VFIO platform.
> 

Is there an document that explains and /or details the various
registers?
> It will do the following:
> 1. Disable/Deactivate each FlexRM ring
> 2. Flush each FlexRM ring
> 
> The cleanup sequence for FlexRM rings is adapted from
> Broadcom FlexRM mailbox driver.
> 
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>  drivers/vfio/platform/reset/Makefile               |   1 +
>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> 
> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> index 70cccc5..392e3c0 100644
> --- a/drivers/vfio/platform/reset/Kconfig
> +++ b/drivers/vfio/platform/reset/Kconfig
> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>  	  Enables the VFIO platform driver to handle reset for AMD XGBE
>  
>  	  If you don't know what to do here, say N.
> +
> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> +	tristate "VFIO support for Broadcom FlexRM reset"
> +	depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> +
> +	  If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
> index 93f4e23..8d9874b 100644
> --- a/drivers/vfio/platform/reset/Makefile
> +++ b/drivers/vfio/platform/reset/Makefile
> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>  
>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> new file mode 100644
> index 0000000..966a813
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> @@ -0,0 +1,100 @@
> +/*
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * This driver provides reset support for Broadcom FlexRM ring manager
> + * to VFIO platform.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include "vfio_platform_private.h"
> +
> +/* FlexRM configuration */
> +#define RING_REGS_SIZE					0x10000
> +#define RING_VER_MAGIC					0x76303031
> +
> +/* Per-Ring register offsets */
> +#define RING_VER					0x000
> +#define RING_CONTROL					0x034
> +#define RING_FLUSH_DONE					0x038
> +
> +/* Register RING_CONTROL fields */
> +#define CONTROL_FLUSH_SHIFT				5
> +
> +/* Register RING_FLUSH_DONE fields */
> +#define FLUSH_DONE_MASK					0x1
> +
> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> +{
> +	unsigned int timeout;
> +
> +	/* Disable/inactivate ring */
> +	writel_relaxed(0x0, ring + RING_CONTROL);
> +
> +	/* Flush ring with timeout of 1s */
> +	timeout = 1000;

Perhaps a #define for this value?

> +	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> +	do {
> +		if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
> +			break;
> +		mdelay(1);
> +	} while (--timeout);
> +
> +	if (!timeout) {
> +		pr_warn("VFIO FlexRM shutdown timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> +{
> +	int rc = 0;
> +	void __iomem *ring;
> +	struct vfio_platform_region *reg = &vdev->regions[0];
> +
> +	/* Map FlexRM ring registers if not mapped */
> +	if (!reg->ioaddr) {
> +		reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
> +		if (!reg->ioaddr)
> +			return -ENOMEM;
> +	}
> +
> +	/* Discover and shutdown each FlexRM ring */
> +	for (ring = reg->ioaddr;
> +	     ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
> +		if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
> +			rc = vfio_platform_bcmflexrm_shutdown(ring);
> +			if (rc)
> +				break;

You sure? Don't you want to continue trying to reset all of them and
then return rc at the end?

> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
> +			  vfio_platform_bcmflexrm_reset);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
> +MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-04  9:50       ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-09-04  9:50 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Will Deacon, Robin Murphy, Joerg Roedel, Baptiste Reynal,
	Alex Williamson, Scott Branden, Linux Kernel, Linux ARM Kernel,
	Linux IOMMU, kvm, BCM Kernel Feedback

Sorry for delayed response...

On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> This patch adds Broadcom FlexRM low-level reset for
>> VFIO platform.
>>
>
> Is there an document that explains and /or details the various
> registers?

Yes, there is a document but its not publicly accessible.

>> It will do the following:
>> 1. Disable/Deactivate each FlexRM ring
>> 2. Flush each FlexRM ring
>>
>> The cleanup sequence for FlexRM rings is adapted from
>> Broadcom FlexRM mailbox driver.
>>
>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>>  drivers/vfio/platform/reset/Makefile               |   1 +
>>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>>  3 files changed, 110 insertions(+)
>>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>>
>> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> index 70cccc5..392e3c0 100644
>> --- a/drivers/vfio/platform/reset/Kconfig
>> +++ b/drivers/vfio/platform/reset/Kconfig
>> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>>         Enables the VFIO platform driver to handle reset for AMD XGBE
>>
>>         If you don't know what to do here, say N.
>> +
>> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> +     tristate "VFIO support for Broadcom FlexRM reset"
>> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> +     default ARCH_BCM_IPROC
>> +     help
>> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> +
>> +       If you don't know what to do here, say N.
>> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> index 93f4e23..8d9874b 100644
>> --- a/drivers/vfio/platform/reset/Makefile
>> +++ b/drivers/vfio/platform/reset/Makefile
>> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>>
>>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> new file mode 100644
>> index 0000000..966a813
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> @@ -0,0 +1,100 @@
>> +/*
>> + * 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.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +/*
>> + * This driver provides reset support for Broadcom FlexRM ring manager
>> + * to VFIO platform.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +#include "vfio_platform_private.h"
>> +
>> +/* FlexRM configuration */
>> +#define RING_REGS_SIZE                                       0x10000
>> +#define RING_VER_MAGIC                                       0x76303031
>> +
>> +/* Per-Ring register offsets */
>> +#define RING_VER                                     0x000
>> +#define RING_CONTROL                                 0x034
>> +#define RING_FLUSH_DONE                                      0x038
>> +
>> +/* Register RING_CONTROL fields */
>> +#define CONTROL_FLUSH_SHIFT                          5
>> +
>> +/* Register RING_FLUSH_DONE fields */
>> +#define FLUSH_DONE_MASK                                      0x1
>> +
>> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> +{
>> +     unsigned int timeout;
>> +
>> +     /* Disable/inactivate ring */
>> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> +
>> +     /* Flush ring with timeout of 1s */
>> +     timeout = 1000;
>
> Perhaps a #define for this value?

This magic value 1000 makes it explicit that we try till 1 second
because inside the loop we have mdelay(1).

>
>> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> +     do {
>> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> +                     break;
>> +             mdelay(1);
>> +     } while (--timeout);
>> +
>> +     if (!timeout) {
>> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>> +             return -ETIMEDOUT;
>> +     }
>> +
>> +     return 0;
>> +}

There is a correction in the sequence suggested by HW folks.

After above, we need to clear the FLUSH bit in RING_CONTROL
register and then wait for FLUSH_DONE bit to be cleared in
RING_FLUSH_DONE register.

This was missed out previously due to incomplete documentation.

>> +
>> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
>> +{
>> +     int rc = 0;
>> +     void __iomem *ring;
>> +     struct vfio_platform_region *reg = &vdev->regions[0];
>> +
>> +     /* Map FlexRM ring registers if not mapped */
>> +     if (!reg->ioaddr) {
>> +             reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
>> +             if (!reg->ioaddr)
>> +                     return -ENOMEM;
>> +     }
>> +
>> +     /* Discover and shutdown each FlexRM ring */
>> +     for (ring = reg->ioaddr;
>> +          ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
>> +             if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
>> +                     rc = vfio_platform_bcmflexrm_shutdown(ring);
>> +                     if (rc)
>> +                             break;
>
> You sure? Don't you want to continue trying to reset all of them and
> then return rc at the end?

Yes, I think its better to reset all even if few fail. I will update this
accordingly.

Thanks,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-04  9:50       ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel via iommu @ 2017-09-04  9:50 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA, Scott Branden, Will Deacon,
	Linux Kernel, Linux IOMMU, BCM Kernel Feedback, Linux ARM Kernel

Sorry for delayed response...

On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
<konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> This patch adds Broadcom FlexRM low-level reset for
>> VFIO platform.
>>
>
> Is there an document that explains and /or details the various
> registers?

Yes, there is a document but its not publicly accessible.

>> It will do the following:
>> 1. Disable/Deactivate each FlexRM ring
>> 2. Flush each FlexRM ring
>>
>> The cleanup sequence for FlexRM rings is adapted from
>> Broadcom FlexRM mailbox driver.
>>
>> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>>  drivers/vfio/platform/reset/Makefile               |   1 +
>>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>>  3 files changed, 110 insertions(+)
>>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>>
>> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> index 70cccc5..392e3c0 100644
>> --- a/drivers/vfio/platform/reset/Kconfig
>> +++ b/drivers/vfio/platform/reset/Kconfig
>> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>>         Enables the VFIO platform driver to handle reset for AMD XGBE
>>
>>         If you don't know what to do here, say N.
>> +
>> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> +     tristate "VFIO support for Broadcom FlexRM reset"
>> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> +     default ARCH_BCM_IPROC
>> +     help
>> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> +
>> +       If you don't know what to do here, say N.
>> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> index 93f4e23..8d9874b 100644
>> --- a/drivers/vfio/platform/reset/Makefile
>> +++ b/drivers/vfio/platform/reset/Makefile
>> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>>
>>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> new file mode 100644
>> index 0000000..966a813
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> @@ -0,0 +1,100 @@
>> +/*
>> + * 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.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +/*
>> + * This driver provides reset support for Broadcom FlexRM ring manager
>> + * to VFIO platform.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +#include "vfio_platform_private.h"
>> +
>> +/* FlexRM configuration */
>> +#define RING_REGS_SIZE                                       0x10000
>> +#define RING_VER_MAGIC                                       0x76303031
>> +
>> +/* Per-Ring register offsets */
>> +#define RING_VER                                     0x000
>> +#define RING_CONTROL                                 0x034
>> +#define RING_FLUSH_DONE                                      0x038
>> +
>> +/* Register RING_CONTROL fields */
>> +#define CONTROL_FLUSH_SHIFT                          5
>> +
>> +/* Register RING_FLUSH_DONE fields */
>> +#define FLUSH_DONE_MASK                                      0x1
>> +
>> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> +{
>> +     unsigned int timeout;
>> +
>> +     /* Disable/inactivate ring */
>> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> +
>> +     /* Flush ring with timeout of 1s */
>> +     timeout = 1000;
>
> Perhaps a #define for this value?

This magic value 1000 makes it explicit that we try till 1 second
because inside the loop we have mdelay(1).

>
>> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> +     do {
>> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> +                     break;
>> +             mdelay(1);
>> +     } while (--timeout);
>> +
>> +     if (!timeout) {
>> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>> +             return -ETIMEDOUT;
>> +     }
>> +
>> +     return 0;
>> +}

There is a correction in the sequence suggested by HW folks.

After above, we need to clear the FLUSH bit in RING_CONTROL
register and then wait for FLUSH_DONE bit to be cleared in
RING_FLUSH_DONE register.

This was missed out previously due to incomplete documentation.

>> +
>> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
>> +{
>> +     int rc = 0;
>> +     void __iomem *ring;
>> +     struct vfio_platform_region *reg = &vdev->regions[0];
>> +
>> +     /* Map FlexRM ring registers if not mapped */
>> +     if (!reg->ioaddr) {
>> +             reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
>> +             if (!reg->ioaddr)
>> +                     return -ENOMEM;
>> +     }
>> +
>> +     /* Discover and shutdown each FlexRM ring */
>> +     for (ring = reg->ioaddr;
>> +          ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
>> +             if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
>> +                     rc = vfio_platform_bcmflexrm_shutdown(ring);
>> +                     if (rc)
>> +                             break;
>
> You sure? Don't you want to continue trying to reset all of them and
> then return rc at the end?

Yes, I think its better to reset all even if few fail. I will update this
accordingly.

Thanks,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-04  9:50       ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-09-04  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

Sorry for delayed response...

On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> This patch adds Broadcom FlexRM low-level reset for
>> VFIO platform.
>>
>
> Is there an document that explains and /or details the various
> registers?

Yes, there is a document but its not publicly accessible.

>> It will do the following:
>> 1. Disable/Deactivate each FlexRM ring
>> 2. Flush each FlexRM ring
>>
>> The cleanup sequence for FlexRM rings is adapted from
>> Broadcom FlexRM mailbox driver.
>>
>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>>  drivers/vfio/platform/reset/Makefile               |   1 +
>>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>>  3 files changed, 110 insertions(+)
>>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>>
>> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> index 70cccc5..392e3c0 100644
>> --- a/drivers/vfio/platform/reset/Kconfig
>> +++ b/drivers/vfio/platform/reset/Kconfig
>> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>>         Enables the VFIO platform driver to handle reset for AMD XGBE
>>
>>         If you don't know what to do here, say N.
>> +
>> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> +     tristate "VFIO support for Broadcom FlexRM reset"
>> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> +     default ARCH_BCM_IPROC
>> +     help
>> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> +
>> +       If you don't know what to do here, say N.
>> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> index 93f4e23..8d9874b 100644
>> --- a/drivers/vfio/platform/reset/Makefile
>> +++ b/drivers/vfio/platform/reset/Makefile
>> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>>
>>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> new file mode 100644
>> index 0000000..966a813
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> @@ -0,0 +1,100 @@
>> +/*
>> + * 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.
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +/*
>> + * This driver provides reset support for Broadcom FlexRM ring manager
>> + * to VFIO platform.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +#include "vfio_platform_private.h"
>> +
>> +/* FlexRM configuration */
>> +#define RING_REGS_SIZE                                       0x10000
>> +#define RING_VER_MAGIC                                       0x76303031
>> +
>> +/* Per-Ring register offsets */
>> +#define RING_VER                                     0x000
>> +#define RING_CONTROL                                 0x034
>> +#define RING_FLUSH_DONE                                      0x038
>> +
>> +/* Register RING_CONTROL fields */
>> +#define CONTROL_FLUSH_SHIFT                          5
>> +
>> +/* Register RING_FLUSH_DONE fields */
>> +#define FLUSH_DONE_MASK                                      0x1
>> +
>> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> +{
>> +     unsigned int timeout;
>> +
>> +     /* Disable/inactivate ring */
>> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> +
>> +     /* Flush ring with timeout of 1s */
>> +     timeout = 1000;
>
> Perhaps a #define for this value?

This magic value 1000 makes it explicit that we try till 1 second
because inside the loop we have mdelay(1).

>
>> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> +     do {
>> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> +                     break;
>> +             mdelay(1);
>> +     } while (--timeout);
>> +
>> +     if (!timeout) {
>> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>> +             return -ETIMEDOUT;
>> +     }
>> +
>> +     return 0;
>> +}

There is a correction in the sequence suggested by HW folks.

After above, we need to clear the FLUSH bit in RING_CONTROL
register and then wait for FLUSH_DONE bit to be cleared in
RING_FLUSH_DONE register.

This was missed out previously due to incomplete documentation.

>> +
>> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
>> +{
>> +     int rc = 0;
>> +     void __iomem *ring;
>> +     struct vfio_platform_region *reg = &vdev->regions[0];
>> +
>> +     /* Map FlexRM ring registers if not mapped */
>> +     if (!reg->ioaddr) {
>> +             reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
>> +             if (!reg->ioaddr)
>> +                     return -ENOMEM;
>> +     }
>> +
>> +     /* Discover and shutdown each FlexRM ring */
>> +     for (ring = reg->ioaddr;
>> +          ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
>> +             if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
>> +                     rc = vfio_platform_bcmflexrm_shutdown(ring);
>> +                     if (rc)
>> +                             break;
>
> You sure? Don't you want to continue trying to reset all of them and
> then return rc at the end?

Yes, I think its better to reset all even if few fail. I will update this
accordingly.

Thanks,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
  2017-09-04  9:50       ` Anup Patel via iommu
@ 2017-09-05 15:57         ` Alex Williamson
  -1 siblings, 0 replies; 17+ messages in thread
From: Alex Williamson @ 2017-09-05 15:57 UTC (permalink / raw)
  To: Anup Patel
  Cc: Konrad Rzeszutek Wilk, Will Deacon, Robin Murphy, Joerg Roedel,
	Baptiste Reynal, Scott Branden, Linux Kernel, Linux ARM Kernel,
	Linux IOMMU, kvm, BCM Kernel Feedback

On Mon, 4 Sep 2017 15:20:11 +0530
Anup Patel <anup.patel@broadcom.com> wrote:

> Sorry for delayed response...
> 
> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:  
> >> This patch adds Broadcom FlexRM low-level reset for
> >> VFIO platform.
> >>  
> >
> > Is there an document that explains and /or details the various
> > registers?  
> 
> Yes, there is a document but its not publicly accessible.
> 
> >> It will do the following:
> >> 1. Disable/Deactivate each FlexRM ring
> >> 2. Flush each FlexRM ring
> >>
> >> The cleanup sequence for FlexRM rings is adapted from
> >> Broadcom FlexRM mailbox driver.
> >>
> >> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> >> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
> >> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >> ---
> >>  drivers/vfio/platform/reset/Kconfig                |   9 ++
> >>  drivers/vfio/platform/reset/Makefile               |   1 +
> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
> >>  3 files changed, 110 insertions(+)
> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >>
> >> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> >> index 70cccc5..392e3c0 100644
> >> --- a/drivers/vfio/platform/reset/Kconfig
> >> +++ b/drivers/vfio/platform/reset/Kconfig
> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
> >>         Enables the VFIO platform driver to handle reset for AMD XGBE
> >>
> >>         If you don't know what to do here, say N.
> >> +
> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> >> +     tristate "VFIO support for Broadcom FlexRM reset"
> >> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> >> +     default ARCH_BCM_IPROC
> >> +     help
> >> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> >> +
> >> +       If you don't know what to do here, say N.
> >> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
> >> index 93f4e23..8d9874b 100644
> >> --- a/drivers/vfio/platform/reset/Makefile
> >> +++ b/drivers/vfio/platform/reset/Makefile
> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
> >>
> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> new file mode 100644
> >> index 0000000..966a813
> >> --- /dev/null
> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> @@ -0,0 +1,100 @@
> >> +/*
> >> + * 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.
> >> + *
> >> + * 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.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +/*
> >> + * This driver provides reset support for Broadcom FlexRM ring manager
> >> + * to VFIO platform.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/init.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +
> >> +#include "vfio_platform_private.h"
> >> +
> >> +/* FlexRM configuration */
> >> +#define RING_REGS_SIZE                                       0x10000
> >> +#define RING_VER_MAGIC                                       0x76303031
> >> +
> >> +/* Per-Ring register offsets */
> >> +#define RING_VER                                     0x000
> >> +#define RING_CONTROL                                 0x034
> >> +#define RING_FLUSH_DONE                                      0x038
> >> +
> >> +/* Register RING_CONTROL fields */
> >> +#define CONTROL_FLUSH_SHIFT                          5
> >> +
> >> +/* Register RING_FLUSH_DONE fields */
> >> +#define FLUSH_DONE_MASK                                      0x1
> >> +
> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> >> +{
> >> +     unsigned int timeout;
> >> +
> >> +     /* Disable/inactivate ring */
> >> +     writel_relaxed(0x0, ring + RING_CONTROL);
> >> +
> >> +     /* Flush ring with timeout of 1s */
> >> +     timeout = 1000;  
> >
> > Perhaps a #define for this value?  
> 
> This magic value 1000 makes it explicit that we try till 1 second
> because inside the loop we have mdelay(1).
> 
> >  
> >> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> >> +     do {
> >> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
> >> +                     break;
> >> +             mdelay(1);
> >> +     } while (--timeout);
> >> +
> >> +     if (!timeout) {
> >> +             pr_warn("VFIO FlexRM shutdown timeout\n");

Since you're planning a new version anyway, perhaps we could also pass
the device through and use a dev_warn here so we can associate this
error to a specific device.  Perhaps even knowing which ring on the
device encountered the timeout would be useful.  Thanks,

Alex

> >> +             return -ETIMEDOUT;
> >> +     }
> >> +
> >> +     return 0;
> >> +}  
> 
> There is a correction in the sequence suggested by HW folks.
> 
> After above, we need to clear the FLUSH bit in RING_CONTROL
> register and then wait for FLUSH_DONE bit to be cleared in
> RING_FLUSH_DONE register.
> 
> This was missed out previously due to incomplete documentation.
> 
> >> +
> >> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> >> +{
> >> +     int rc = 0;
> >> +     void __iomem *ring;
> >> +     struct vfio_platform_region *reg = &vdev->regions[0];
> >> +
> >> +     /* Map FlexRM ring registers if not mapped */
> >> +     if (!reg->ioaddr) {
> >> +             reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
> >> +             if (!reg->ioaddr)
> >> +                     return -ENOMEM;
> >> +     }
> >> +
> >> +     /* Discover and shutdown each FlexRM ring */
> >> +     for (ring = reg->ioaddr;
> >> +          ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
> >> +             if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
> >> +                     rc = vfio_platform_bcmflexrm_shutdown(ring);
> >> +                     if (rc)
> >> +                             break;  
> >
> > You sure? Don't you want to continue trying to reset all of them and
> > then return rc at the end?  
> 
> Yes, I think its better to reset all even if few fail. I will update this
> accordingly.
> 
> Thanks,
> Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-05 15:57         ` Alex Williamson
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Williamson @ 2017-09-05 15:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 4 Sep 2017 15:20:11 +0530
Anup Patel <anup.patel@broadcom.com> wrote:

> Sorry for delayed response...
> 
> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:  
> >> This patch adds Broadcom FlexRM low-level reset for
> >> VFIO platform.
> >>  
> >
> > Is there an document that explains and /or details the various
> > registers?  
> 
> Yes, there is a document but its not publicly accessible.
> 
> >> It will do the following:
> >> 1. Disable/Deactivate each FlexRM ring
> >> 2. Flush each FlexRM ring
> >>
> >> The cleanup sequence for FlexRM rings is adapted from
> >> Broadcom FlexRM mailbox driver.
> >>
> >> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> >> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
> >> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >> ---
> >>  drivers/vfio/platform/reset/Kconfig                |   9 ++
> >>  drivers/vfio/platform/reset/Makefile               |   1 +
> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
> >>  3 files changed, 110 insertions(+)
> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >>
> >> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> >> index 70cccc5..392e3c0 100644
> >> --- a/drivers/vfio/platform/reset/Kconfig
> >> +++ b/drivers/vfio/platform/reset/Kconfig
> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
> >>         Enables the VFIO platform driver to handle reset for AMD XGBE
> >>
> >>         If you don't know what to do here, say N.
> >> +
> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> >> +     tristate "VFIO support for Broadcom FlexRM reset"
> >> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> >> +     default ARCH_BCM_IPROC
> >> +     help
> >> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> >> +
> >> +       If you don't know what to do here, say N.
> >> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
> >> index 93f4e23..8d9874b 100644
> >> --- a/drivers/vfio/platform/reset/Makefile
> >> +++ b/drivers/vfio/platform/reset/Makefile
> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
> >>
> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> new file mode 100644
> >> index 0000000..966a813
> >> --- /dev/null
> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> >> @@ -0,0 +1,100 @@
> >> +/*
> >> + * 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.
> >> + *
> >> + * 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.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +/*
> >> + * This driver provides reset support for Broadcom FlexRM ring manager
> >> + * to VFIO platform.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/init.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +
> >> +#include "vfio_platform_private.h"
> >> +
> >> +/* FlexRM configuration */
> >> +#define RING_REGS_SIZE                                       0x10000
> >> +#define RING_VER_MAGIC                                       0x76303031
> >> +
> >> +/* Per-Ring register offsets */
> >> +#define RING_VER                                     0x000
> >> +#define RING_CONTROL                                 0x034
> >> +#define RING_FLUSH_DONE                                      0x038
> >> +
> >> +/* Register RING_CONTROL fields */
> >> +#define CONTROL_FLUSH_SHIFT                          5
> >> +
> >> +/* Register RING_FLUSH_DONE fields */
> >> +#define FLUSH_DONE_MASK                                      0x1
> >> +
> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> >> +{
> >> +     unsigned int timeout;
> >> +
> >> +     /* Disable/inactivate ring */
> >> +     writel_relaxed(0x0, ring + RING_CONTROL);
> >> +
> >> +     /* Flush ring with timeout of 1s */
> >> +     timeout = 1000;  
> >
> > Perhaps a #define for this value?  
> 
> This magic value 1000 makes it explicit that we try till 1 second
> because inside the loop we have mdelay(1).
> 
> >  
> >> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> >> +     do {
> >> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
> >> +                     break;
> >> +             mdelay(1);
> >> +     } while (--timeout);
> >> +
> >> +     if (!timeout) {
> >> +             pr_warn("VFIO FlexRM shutdown timeout\n");

Since you're planning a new version anyway, perhaps we could also pass
the device through and use a dev_warn here so we can associate this
error to a specific device.  Perhaps even knowing which ring on the
device encountered the timeout would be useful.  Thanks,

Alex

> >> +             return -ETIMEDOUT;
> >> +     }
> >> +
> >> +     return 0;
> >> +}  
> 
> There is a correction in the sequence suggested by HW folks.
> 
> After above, we need to clear the FLUSH bit in RING_CONTROL
> register and then wait for FLUSH_DONE bit to be cleared in
> RING_FLUSH_DONE register.
> 
> This was missed out previously due to incomplete documentation.
> 
> >> +
> >> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> >> +{
> >> +     int rc = 0;
> >> +     void __iomem *ring;
> >> +     struct vfio_platform_region *reg = &vdev->regions[0];
> >> +
> >> +     /* Map FlexRM ring registers if not mapped */
> >> +     if (!reg->ioaddr) {
> >> +             reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
> >> +             if (!reg->ioaddr)
> >> +                     return -ENOMEM;
> >> +     }
> >> +
> >> +     /* Discover and shutdown each FlexRM ring */
> >> +     for (ring = reg->ioaddr;
> >> +          ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
> >> +             if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC) {
> >> +                     rc = vfio_platform_bcmflexrm_shutdown(ring);
> >> +                     if (rc)
> >> +                             break;  
> >
> > You sure? Don't you want to continue trying to reset all of them and
> > then return rc at the end?  
> 
> Yes, I think its better to reset all even if few fail. I will update this
> accordingly.
> 
> Thanks,
> Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-06  8:14           ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-09-06  8:14 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Konrad Rzeszutek Wilk, Will Deacon, Robin Murphy, Joerg Roedel,
	Baptiste Reynal, Scott Branden, Linux Kernel, Linux ARM Kernel,
	Linux IOMMU, kvm, BCM Kernel Feedback

On Tue, Sep 5, 2017 at 9:27 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> On Mon, 4 Sep 2017 15:20:11 +0530
> Anup Patel <anup.patel@broadcom.com> wrote:
>
>> Sorry for delayed response...
>>
>> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> >> This patch adds Broadcom FlexRM low-level reset for
>> >> VFIO platform.
>> >>
>> >
>> > Is there an document that explains and /or details the various
>> > registers?
>>
>> Yes, there is a document but its not publicly accessible.
>>
>> >> It will do the following:
>> >> 1. Disable/Deactivate each FlexRM ring
>> >> 2. Flush each FlexRM ring
>> >>
>> >> The cleanup sequence for FlexRM rings is adapted from
>> >> Broadcom FlexRM mailbox driver.
>> >>
>> >> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> >> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
>> >> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> >> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> >> ---
>> >>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>> >>  drivers/vfio/platform/reset/Makefile               |   1 +
>> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>> >>  3 files changed, 110 insertions(+)
>> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >>
>> >> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> >> index 70cccc5..392e3c0 100644
>> >> --- a/drivers/vfio/platform/reset/Kconfig
>> >> +++ b/drivers/vfio/platform/reset/Kconfig
>> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>> >>         Enables the VFIO platform driver to handle reset for AMD XGBE
>> >>
>> >>         If you don't know what to do here, say N.
>> >> +
>> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> >> +     tristate "VFIO support for Broadcom FlexRM reset"
>> >> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> >> +     default ARCH_BCM_IPROC
>> >> +     help
>> >> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> >> +
>> >> +       If you don't know what to do here, say N.
>> >> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> >> index 93f4e23..8d9874b 100644
>> >> --- a/drivers/vfio/platform/reset/Makefile
>> >> +++ b/drivers/vfio/platform/reset/Makefile
>> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>> >>
>> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> new file mode 100644
>> >> index 0000000..966a813
>> >> --- /dev/null
>> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> @@ -0,0 +1,100 @@
>> >> +/*
>> >> + * 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.
>> >> + *
>> >> + * 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.
>> >> + *
>> >> + * You should have received a copy of the GNU General Public License
>> >> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> >> + */
>> >> +
>> >> +/*
>> >> + * This driver provides reset support for Broadcom FlexRM ring manager
>> >> + * to VFIO platform.
>> >> + */
>> >> +
>> >> +#include <linux/delay.h>
>> >> +#include <linux/init.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +
>> >> +#include "vfio_platform_private.h"
>> >> +
>> >> +/* FlexRM configuration */
>> >> +#define RING_REGS_SIZE                                       0x10000
>> >> +#define RING_VER_MAGIC                                       0x76303031
>> >> +
>> >> +/* Per-Ring register offsets */
>> >> +#define RING_VER                                     0x000
>> >> +#define RING_CONTROL                                 0x034
>> >> +#define RING_FLUSH_DONE                                      0x038
>> >> +
>> >> +/* Register RING_CONTROL fields */
>> >> +#define CONTROL_FLUSH_SHIFT                          5
>> >> +
>> >> +/* Register RING_FLUSH_DONE fields */
>> >> +#define FLUSH_DONE_MASK                                      0x1
>> >> +
>> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> >> +{
>> >> +     unsigned int timeout;
>> >> +
>> >> +     /* Disable/inactivate ring */
>> >> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> >> +
>> >> +     /* Flush ring with timeout of 1s */
>> >> +     timeout = 1000;
>> >
>> > Perhaps a #define for this value?
>>
>> This magic value 1000 makes it explicit that we try till 1 second
>> because inside the loop we have mdelay(1).
>>
>> >
>> >> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> >> +     do {
>> >> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> >> +                     break;
>> >> +             mdelay(1);
>> >> +     } while (--timeout);
>> >> +
>> >> +     if (!timeout) {
>> >> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>
> Since you're planning a new version anyway, perhaps we could also pass
> the device through and use a dev_warn here so we can associate this
> error to a specific device.  Perhaps even knowing which ring on the
> device encountered the timeout would be useful.  Thanks,

Sure, will do.

Regards,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-06  8:14           ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel via iommu @ 2017-09-06  8:14 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA, Scott Branden, Konrad Rzeszutek Wilk,
	Will Deacon, Linux Kernel, Linux IOMMU, BCM Kernel Feedback,
	Linux ARM Kernel

On Tue, Sep 5, 2017 at 9:27 PM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 4 Sep 2017 15:20:11 +0530
> Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>
>> Sorry for delayed response...
>>
>> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
>> <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> >> This patch adds Broadcom FlexRM low-level reset for
>> >> VFIO platform.
>> >>
>> >
>> > Is there an document that explains and /or details the various
>> > registers?
>>
>> Yes, there is a document but its not publicly accessible.
>>
>> >> It will do the following:
>> >> 1. Disable/Deactivate each FlexRM ring
>> >> 2. Flush each FlexRM ring
>> >>
>> >> The cleanup sequence for FlexRM rings is adapted from
>> >> Broadcom FlexRM mailbox driver.
>> >>
>> >> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >> Reviewed-by: Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >> Reviewed-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> >> ---
>> >>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>> >>  drivers/vfio/platform/reset/Makefile               |   1 +
>> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>> >>  3 files changed, 110 insertions(+)
>> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >>
>> >> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> >> index 70cccc5..392e3c0 100644
>> >> --- a/drivers/vfio/platform/reset/Kconfig
>> >> +++ b/drivers/vfio/platform/reset/Kconfig
>> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>> >>         Enables the VFIO platform driver to handle reset for AMD XGBE
>> >>
>> >>         If you don't know what to do here, say N.
>> >> +
>> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> >> +     tristate "VFIO support for Broadcom FlexRM reset"
>> >> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> >> +     default ARCH_BCM_IPROC
>> >> +     help
>> >> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> >> +
>> >> +       If you don't know what to do here, say N.
>> >> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> >> index 93f4e23..8d9874b 100644
>> >> --- a/drivers/vfio/platform/reset/Makefile
>> >> +++ b/drivers/vfio/platform/reset/Makefile
>> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>> >>
>> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> new file mode 100644
>> >> index 0000000..966a813
>> >> --- /dev/null
>> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> @@ -0,0 +1,100 @@
>> >> +/*
>> >> + * 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.
>> >> + *
>> >> + * 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.
>> >> + *
>> >> + * You should have received a copy of the GNU General Public License
>> >> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> >> + */
>> >> +
>> >> +/*
>> >> + * This driver provides reset support for Broadcom FlexRM ring manager
>> >> + * to VFIO platform.
>> >> + */
>> >> +
>> >> +#include <linux/delay.h>
>> >> +#include <linux/init.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +
>> >> +#include "vfio_platform_private.h"
>> >> +
>> >> +/* FlexRM configuration */
>> >> +#define RING_REGS_SIZE                                       0x10000
>> >> +#define RING_VER_MAGIC                                       0x76303031
>> >> +
>> >> +/* Per-Ring register offsets */
>> >> +#define RING_VER                                     0x000
>> >> +#define RING_CONTROL                                 0x034
>> >> +#define RING_FLUSH_DONE                                      0x038
>> >> +
>> >> +/* Register RING_CONTROL fields */
>> >> +#define CONTROL_FLUSH_SHIFT                          5
>> >> +
>> >> +/* Register RING_FLUSH_DONE fields */
>> >> +#define FLUSH_DONE_MASK                                      0x1
>> >> +
>> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> >> +{
>> >> +     unsigned int timeout;
>> >> +
>> >> +     /* Disable/inactivate ring */
>> >> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> >> +
>> >> +     /* Flush ring with timeout of 1s */
>> >> +     timeout = 1000;
>> >
>> > Perhaps a #define for this value?
>>
>> This magic value 1000 makes it explicit that we try till 1 second
>> because inside the loop we have mdelay(1).
>>
>> >
>> >> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> >> +     do {
>> >> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> >> +                     break;
>> >> +             mdelay(1);
>> >> +     } while (--timeout);
>> >> +
>> >> +     if (!timeout) {
>> >> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>
> Since you're planning a new version anyway, perhaps we could also pass
> the device through and use a dev_warn here so we can associate this
> error to a specific device.  Perhaps even knowing which ring on the
> device encountered the timeout would be useful.  Thanks,

Sure, will do.

Regards,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module
@ 2017-09-06  8:14           ` Anup Patel via iommu
  0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2017-09-06  8:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 5, 2017 at 9:27 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> On Mon, 4 Sep 2017 15:20:11 +0530
> Anup Patel <anup.patel@broadcom.com> wrote:
>
>> Sorry for delayed response...
>>
>> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> >> This patch adds Broadcom FlexRM low-level reset for
>> >> VFIO platform.
>> >>
>> >
>> > Is there an document that explains and /or details the various
>> > registers?
>>
>> Yes, there is a document but its not publicly accessible.
>>
>> >> It will do the following:
>> >> 1. Disable/Deactivate each FlexRM ring
>> >> 2. Flush each FlexRM ring
>> >>
>> >> The cleanup sequence for FlexRM rings is adapted from
>> >> Broadcom FlexRM mailbox driver.
>> >>
>> >> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> >> Reviewed-by: Oza Oza <oza.oza@broadcom.com>
>> >> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> >> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>> >> ---
>> >>  drivers/vfio/platform/reset/Kconfig                |   9 ++
>> >>  drivers/vfio/platform/reset/Makefile               |   1 +
>> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 +++++++++++++++++++++
>> >>  3 files changed, 110 insertions(+)
>> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >>
>> >> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> >> index 70cccc5..392e3c0 100644
>> >> --- a/drivers/vfio/platform/reset/Kconfig
>> >> +++ b/drivers/vfio/platform/reset/Kconfig
>> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>> >>         Enables the VFIO platform driver to handle reset for AMD XGBE
>> >>
>> >>         If you don't know what to do here, say N.
>> >> +
>> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> >> +     tristate "VFIO support for Broadcom FlexRM reset"
>> >> +     depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> >> +     default ARCH_BCM_IPROC
>> >> +     help
>> >> +       Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> >> +
>> >> +       If you don't know what to do here, say N.
>> >> diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
>> >> index 93f4e23..8d9874b 100644
>> >> --- a/drivers/vfio/platform/reset/Makefile
>> >> +++ b/drivers/vfio/platform/reset/Makefile
>> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>> >>
>> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
>> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> new file mode 100644
>> >> index 0000000..966a813
>> >> --- /dev/null
>> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> @@ -0,0 +1,100 @@
>> >> +/*
>> >> + * 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.
>> >> + *
>> >> + * 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.
>> >> + *
>> >> + * You should have received a copy of the GNU General Public License
>> >> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> >> + */
>> >> +
>> >> +/*
>> >> + * This driver provides reset support for Broadcom FlexRM ring manager
>> >> + * to VFIO platform.
>> >> + */
>> >> +
>> >> +#include <linux/delay.h>
>> >> +#include <linux/init.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +
>> >> +#include "vfio_platform_private.h"
>> >> +
>> >> +/* FlexRM configuration */
>> >> +#define RING_REGS_SIZE                                       0x10000
>> >> +#define RING_VER_MAGIC                                       0x76303031
>> >> +
>> >> +/* Per-Ring register offsets */
>> >> +#define RING_VER                                     0x000
>> >> +#define RING_CONTROL                                 0x034
>> >> +#define RING_FLUSH_DONE                                      0x038
>> >> +
>> >> +/* Register RING_CONTROL fields */
>> >> +#define CONTROL_FLUSH_SHIFT                          5
>> >> +
>> >> +/* Register RING_FLUSH_DONE fields */
>> >> +#define FLUSH_DONE_MASK                                      0x1
>> >> +
>> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> >> +{
>> >> +     unsigned int timeout;
>> >> +
>> >> +     /* Disable/inactivate ring */
>> >> +     writel_relaxed(0x0, ring + RING_CONTROL);
>> >> +
>> >> +     /* Flush ring with timeout of 1s */
>> >> +     timeout = 1000;
>> >
>> > Perhaps a #define for this value?
>>
>> This magic value 1000 makes it explicit that we try till 1 second
>> because inside the loop we have mdelay(1).
>>
>> >
>> >> +     writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> >> +     do {
>> >> +             if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
>> >> +                     break;
>> >> +             mdelay(1);
>> >> +     } while (--timeout);
>> >> +
>> >> +     if (!timeout) {
>> >> +             pr_warn("VFIO FlexRM shutdown timeout\n");
>
> Since you're planning a new version anyway, perhaps we could also pass
> the device through and use a dev_warn here so we can associate this
> error to a specific device.  Perhaps even knowing which ring on the
> device encountered the timeout would be useful.  Thanks,

Sure, will do.

Regards,
Anup

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-09-06  8:15 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29  4:04 [PATCH v6] FlexRM support in VFIO platform Anup Patel
2017-08-29  4:04 ` Anup Patel
2017-08-29  4:04 ` Anup Patel via iommu
2017-08-29  4:04 ` [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module Anup Patel
2017-08-29  4:04   ` Anup Patel
2017-08-29  4:04   ` Anup Patel via iommu
2017-08-29 14:09   ` Konrad Rzeszutek Wilk
2017-08-29 14:09     ` Konrad Rzeszutek Wilk
2017-08-29 14:09     ` Konrad Rzeszutek Wilk
2017-09-04  9:50     ` Anup Patel
2017-09-04  9:50       ` Anup Patel
2017-09-04  9:50       ` Anup Patel via iommu
2017-09-05 15:57       ` Alex Williamson
2017-09-05 15:57         ` Alex Williamson
2017-09-06  8:14         ` Anup Patel
2017-09-06  8:14           ` Anup Patel
2017-09-06  8:14           ` Anup Patel via iommu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.