All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
       [not found] <20220609102855.272270-1-wenhu.wang@hotmail.com>
@ 2022-06-09 10:28   ` Wang Wenhu
  2022-06-09 10:28   ` Wang Wenhu
  1 sibling, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-09 10:28 UTC (permalink / raw)
  To: gregkh, christophe.leroy; +Cc: mpe, linuxppc-dev, linux-kernel, Wang Wenhu

The function ioremap_coherent may be called by modules such as
fsl_85xx_cache_sram. So export it for access in other modules.

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
 arch/powerpc/mm/ioremap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
index 4f12504fb405..08a00dacef0b 100644
--- a/arch/powerpc/mm/ioremap.c
+++ b/arch/powerpc/mm/ioremap.c
@@ -40,6 +40,7 @@ void __iomem *ioremap_coherent(phys_addr_t addr, unsigned long size)
 		return iowa_ioremap(addr, size, prot, caller);
 	return __ioremap_caller(addr, size, prot, caller);
 }
+EXPORT_SYMBOL(ioremap_coherent);
 
 void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
@ 2022-06-09 10:28   ` Wang Wenhu
  0 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-09 10:28 UTC (permalink / raw)
  To: gregkh, christophe.leroy; +Cc: linuxppc-dev, linux-kernel, Wang Wenhu

The function ioremap_coherent may be called by modules such as
fsl_85xx_cache_sram. So export it for access in other modules.

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
 arch/powerpc/mm/ioremap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
index 4f12504fb405..08a00dacef0b 100644
--- a/arch/powerpc/mm/ioremap.c
+++ b/arch/powerpc/mm/ioremap.c
@@ -40,6 +40,7 @@ void __iomem *ioremap_coherent(phys_addr_t addr, unsigned long size)
 		return iowa_ioremap(addr, size, prot, caller);
 	return __ioremap_caller(addr, size, prot, caller);
 }
+EXPORT_SYMBOL(ioremap_coherent);
 
 void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
       [not found] <20220609102855.272270-1-wenhu.wang@hotmail.com>
@ 2022-06-09 10:28   ` Wang Wenhu
  2022-06-09 10:28   ` Wang Wenhu
  1 sibling, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-09 10:28 UTC (permalink / raw)
  To: gregkh, christophe.leroy; +Cc: mpe, linuxppc-dev, linux-kernel, Wang Wenhu

The l2-cache could be optionally configured as SRAM partly or fully.
Users can make use of it as a block of independent memory that offers
special usage, such as for debuging or other cratical status info
storage which keeps consistently even when the whole system crashed.

The hardware related configuration process utilized the work of the
earlier implementation, which has been removed now.
See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03

Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
 drivers/uio/Kconfig                   |  10 +
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
 3 files changed, 297 insertions(+)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 2e16c5338e5b..9199ced03880 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,16 @@ config UIO_NETX
 	  To compile this driver as a module, choose M here; the module
 	  will be called uio_netx.
 
+config UIO_FSL_85XX_CACHE_SRAM
+	tristate "Freescale 85xx Cache-Sram driver"
+	depends on FSL_SOC_BOOKE && PPC32
+	help
+	  Generic driver for accessing the Cache-Sram form user level. This
+	  is extremely helpful for some user-space applications that require
+	  high performance memory accesses.
+
+	  If you don't know what to do here, say N.
+
 config UIO_FSL_ELBC_GPCM
 	tristate "eLBC/GPCM driver"
 	depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index f2f416a14228..1ba07d92a1b1 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
 obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index 000000000000..d363f9d2b179
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
+ * All rights reserved.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/stringify.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME		"uio_mpc85xx_cache_sram"
+#define UIO_INFO_VER	"0.0.1"
+#define UIO_NAME		"uio_cache_sram"
+
+#define L2CR_L2FI				0x40000000	/* L2 flash invalidate */
+#define L2CR_L2IO				0x00200000	/* L2 instruction only */
+#define L2CR_SRAM_ZERO			0x00000000	/* L2SRAM zero size */
+#define L2CR_SRAM_FULL			0x00010000	/* L2SRAM full size */
+#define L2CR_SRAM_HALF			0x00020000	/* L2SRAM half size */
+#define L2CR_SRAM_TWO_HALFS		0x00030000	/* L2SRAM two half sizes */
+#define L2CR_SRAM_QUART			0x00040000	/* L2SRAM one quarter size */
+#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
+#define L2CR_SRAM_EIGHTH		0x00060000	/* L2SRAM one eighth size */
+#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
+
+#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
+
+#define L2SRAM_BAR_MSK_LO18		0xFFFFC000	/* Lower 18 bits */
+#define L2SRAM_BARE_MSK_HI4		0x0000000F	/* Upper 4 bits */
+
+enum cache_sram_lock_ways {
+	LOCK_WAYS_ZERO,
+	LOCK_WAYS_EIGHTH,
+	LOCK_WAYS_TWO_EIGHTH,
+	LOCK_WAYS_HALF = 4,
+	LOCK_WAYS_FULL = 8,
+};
+
+struct mpc85xx_l2ctlr {
+	u32	ctl;		/* 0x000 - L2 control */
+	u8	res1[0xC];
+	u32	ewar0;		/* 0x010 - External write address 0 */
+	u32	ewarea0;	/* 0x014 - External write address extended 0 */
+	u32	ewcr0;		/* 0x018 - External write ctrl */
+	u8	res2[4];
+	u32	ewar1;		/* 0x020 - External write address 1 */
+	u32	ewarea1;	/* 0x024 - External write address extended 1 */
+	u32	ewcr1;		/* 0x028 - External write ctrl 1 */
+	u8	res3[4];
+	u32	ewar2;		/* 0x030 - External write address 2 */
+	u32	ewarea2;	/* 0x034 - External write address extended 2 */
+	u32	ewcr2;		/* 0x038 - External write ctrl 2 */
+	u8	res4[4];
+	u32	ewar3;		/* 0x040 - External write address 3 */
+	u32	ewarea3;	/* 0x044 - External write address extended 3 */
+	u32	ewcr3;		/* 0x048 - External write ctrl 3 */
+	u8	res5[0xB4];
+	u32	srbar0;		/* 0x100 - SRAM base address 0 */
+	u32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
+	u32	srbar1;		/* 0x108 - SRAM base address 1 */
+	u32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
+	u8	res6[0xCF0];
+	u32	errinjhi;	/* 0xE00 - Error injection mask high */
+	u32	errinjlo;	/* 0xE04 - Error injection mask low */
+	u32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
+	u8	res7[0x14];
+	u32	captdatahi;	/* 0xE20 - Error data high capture */
+	u32	captdatalo;	/* 0xE24 - Error data low capture */
+	u32	captecc;	/* 0xE28 - Error syndrome */
+	u8	res8[0x14];
+	u32	errdet;		/* 0xE40 - Error detect */
+	u32	errdis;		/* 0xE44 - Error disable */
+	u32	errinten;	/* 0xE48 - Error interrupt enable */
+	u32	errattr;	/* 0xE4c - Error attribute capture */
+	u32	erradrrl;	/* 0xE50 - Error address capture low */
+	u32	erradrrh;	/* 0xE54 - Error address capture high */
+	u32	errctl;		/* 0xE58 - Error control */
+	u8	res9[0x1A4];
+};
+
+static int uio_cache_sram_setup(struct platform_device *pdev,
+				phys_addr_t base, u8 ways)
+{
+	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
+
+	if (!l2ctlr) {
+		dev_err(&pdev->dev, "can not map l2 controller\n");
+		return -EINVAL;
+	}
+
+	/* write bits[0-17] to srbar0 */
+	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
+
+	/* write bits[18-21] to srbare0 */
+#ifdef CONFIG_PHYS_64BIT
+	out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
+#endif
+
+	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
+
+	switch (ways) {
+	case LOCK_WAYS_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
+		break;
+
+	case LOCK_WAYS_TWO_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
+		break;
+
+	case LOCK_WAYS_HALF:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
+		break;
+
+	case LOCK_WAYS_FULL:
+	default:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
+		break;
+	}
+	eieio();
+
+	return 0;
+}
+
+static const struct vm_operations_struct uio_cache_sram_vm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+	.access = generic_access_phys,
+#endif
+};
+
+static int uio_cache_sram_mmap(struct uio_info *info,
+				struct vm_area_struct *vma)
+{
+	struct uio_mem *mem = info->mem;
+
+	if (mem->addr & ~PAGE_MASK)
+		return -ENODEV;
+
+	if ((vma->vm_end - vma->vm_start > mem->size) ||
+		(mem->size == 0) ||
+		(mem->memtype != UIO_MEM_PHYS))
+		return -EINVAL;
+
+	vma->vm_ops = &uio_cache_sram_vm_ops;
+	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
+
+	return remap_pfn_range(vma,
+						   vma->vm_start,
+						   mem->addr >> PAGE_SHIFT,
+						   vma->vm_end - vma->vm_start,
+						   vma->vm_page_prot);
+}
+
+static int uio_cache_sram_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct uio_info *info;
+	struct uio_mem *uiomem;
+	const char *dt_name;
+	phys_addr_t mem_base;
+	u32 l2cache_size;
+	u32 mem_size;
+	u32 rem;
+	u8 ways;
+	int ret;
+
+	if (!node) {
+		dev_err(&pdev->dev, "device's of_node is null\n");
+		return -EINVAL;
+	}
+
+	/* alloc uio_info for one device */
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	/* get optional uio name */
+	if (of_property_read_string(node, "uio_name", &dt_name))
+		dt_name = UIO_NAME;
+
+	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
+	if (!info->name)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-size\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-mem-base", &mem_base);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-base\n");
+		return -EINVAL;
+	}
+
+	if (mem_size == 0) {
+		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing l2cache-size\n");
+		return -EINVAL;
+	}
+
+	rem = l2cache_size % mem_size;
+	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
+	if (rem || (ways & (ways - 1))) {
+		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
+		return -EINVAL;
+	}
+
+	ret = uio_cache_sram_setup(pdev, mem_base, ways);
+	if (ret)
+		return ret;
+
+	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
+		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
+		ret = -ENXIO;
+	}
+
+	info->irq = UIO_IRQ_NONE;
+	info->version = UIO_INFO_VER;
+	info->mmap = uio_cache_sram_mmap;
+	uiomem = info->mem;
+	uiomem->memtype = UIO_MEM_PHYS;
+	uiomem->addr = mem_base;
+	uiomem->size = mem_size;
+	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
+	uiomem->internal_addr = ioremap_coherent(mem_base, mem_size);
+	if (!uiomem->internal_addr) {
+		dev_err(&pdev->dev, "cache ioremep_coherent failed\n");
+		ret = -ENOMEM;
+	}
+
+	/* register uio device */
+	if (uio_register_device(&pdev->dev, info)) {
+		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
+		ret = -ENODEV;
+		goto err_out;
+	}
+
+	platform_set_drvdata(pdev, info);
+	return 0;
+
+err_out:
+	iounmap(info->mem[0].internal_addr);
+	return ret;
+}
+
+static int uio_cache_sram_remove(struct platform_device *pdev)
+{
+	struct uio_info *info = platform_get_drvdata(pdev);
+
+	uio_unregister_device(info);
+	iounmap(info->mem[0].internal_addr);
+
+	return 0;
+}
+
+static const struct of_device_id uio_cache_sram_of_match[] = {
+	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
+
+static struct platform_driver uio_fsl_85xx_cache_sram = {
+	.probe = uio_cache_sram_probe,
+	.remove = uio_cache_sram_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table	= uio_cache_sram_of_match,
+	},
+};
+
+module_platform_driver(uio_fsl_85xx_cache_sram);
+
+MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
+MODULE_DESCRIPTION("Freescale MPC85xx Cache-Sram UIO Platform Driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-09 10:28   ` Wang Wenhu
  0 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-09 10:28 UTC (permalink / raw)
  To: gregkh, christophe.leroy; +Cc: linuxppc-dev, linux-kernel, Wang Wenhu

The l2-cache could be optionally configured as SRAM partly or fully.
Users can make use of it as a block of independent memory that offers
special usage, such as for debuging or other cratical status info
storage which keeps consistently even when the whole system crashed.

The hardware related configuration process utilized the work of the
earlier implementation, which has been removed now.
See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03

Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
 drivers/uio/Kconfig                   |  10 +
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
 3 files changed, 297 insertions(+)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 2e16c5338e5b..9199ced03880 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,16 @@ config UIO_NETX
 	  To compile this driver as a module, choose M here; the module
 	  will be called uio_netx.
 
+config UIO_FSL_85XX_CACHE_SRAM
+	tristate "Freescale 85xx Cache-Sram driver"
+	depends on FSL_SOC_BOOKE && PPC32
+	help
+	  Generic driver for accessing the Cache-Sram form user level. This
+	  is extremely helpful for some user-space applications that require
+	  high performance memory accesses.
+
+	  If you don't know what to do here, say N.
+
 config UIO_FSL_ELBC_GPCM
 	tristate "eLBC/GPCM driver"
 	depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index f2f416a14228..1ba07d92a1b1 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
 obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index 000000000000..d363f9d2b179
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
+ * All rights reserved.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/stringify.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME		"uio_mpc85xx_cache_sram"
+#define UIO_INFO_VER	"0.0.1"
+#define UIO_NAME		"uio_cache_sram"
+
+#define L2CR_L2FI				0x40000000	/* L2 flash invalidate */
+#define L2CR_L2IO				0x00200000	/* L2 instruction only */
+#define L2CR_SRAM_ZERO			0x00000000	/* L2SRAM zero size */
+#define L2CR_SRAM_FULL			0x00010000	/* L2SRAM full size */
+#define L2CR_SRAM_HALF			0x00020000	/* L2SRAM half size */
+#define L2CR_SRAM_TWO_HALFS		0x00030000	/* L2SRAM two half sizes */
+#define L2CR_SRAM_QUART			0x00040000	/* L2SRAM one quarter size */
+#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
+#define L2CR_SRAM_EIGHTH		0x00060000	/* L2SRAM one eighth size */
+#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
+
+#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
+
+#define L2SRAM_BAR_MSK_LO18		0xFFFFC000	/* Lower 18 bits */
+#define L2SRAM_BARE_MSK_HI4		0x0000000F	/* Upper 4 bits */
+
+enum cache_sram_lock_ways {
+	LOCK_WAYS_ZERO,
+	LOCK_WAYS_EIGHTH,
+	LOCK_WAYS_TWO_EIGHTH,
+	LOCK_WAYS_HALF = 4,
+	LOCK_WAYS_FULL = 8,
+};
+
+struct mpc85xx_l2ctlr {
+	u32	ctl;		/* 0x000 - L2 control */
+	u8	res1[0xC];
+	u32	ewar0;		/* 0x010 - External write address 0 */
+	u32	ewarea0;	/* 0x014 - External write address extended 0 */
+	u32	ewcr0;		/* 0x018 - External write ctrl */
+	u8	res2[4];
+	u32	ewar1;		/* 0x020 - External write address 1 */
+	u32	ewarea1;	/* 0x024 - External write address extended 1 */
+	u32	ewcr1;		/* 0x028 - External write ctrl 1 */
+	u8	res3[4];
+	u32	ewar2;		/* 0x030 - External write address 2 */
+	u32	ewarea2;	/* 0x034 - External write address extended 2 */
+	u32	ewcr2;		/* 0x038 - External write ctrl 2 */
+	u8	res4[4];
+	u32	ewar3;		/* 0x040 - External write address 3 */
+	u32	ewarea3;	/* 0x044 - External write address extended 3 */
+	u32	ewcr3;		/* 0x048 - External write ctrl 3 */
+	u8	res5[0xB4];
+	u32	srbar0;		/* 0x100 - SRAM base address 0 */
+	u32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
+	u32	srbar1;		/* 0x108 - SRAM base address 1 */
+	u32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
+	u8	res6[0xCF0];
+	u32	errinjhi;	/* 0xE00 - Error injection mask high */
+	u32	errinjlo;	/* 0xE04 - Error injection mask low */
+	u32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
+	u8	res7[0x14];
+	u32	captdatahi;	/* 0xE20 - Error data high capture */
+	u32	captdatalo;	/* 0xE24 - Error data low capture */
+	u32	captecc;	/* 0xE28 - Error syndrome */
+	u8	res8[0x14];
+	u32	errdet;		/* 0xE40 - Error detect */
+	u32	errdis;		/* 0xE44 - Error disable */
+	u32	errinten;	/* 0xE48 - Error interrupt enable */
+	u32	errattr;	/* 0xE4c - Error attribute capture */
+	u32	erradrrl;	/* 0xE50 - Error address capture low */
+	u32	erradrrh;	/* 0xE54 - Error address capture high */
+	u32	errctl;		/* 0xE58 - Error control */
+	u8	res9[0x1A4];
+};
+
+static int uio_cache_sram_setup(struct platform_device *pdev,
+				phys_addr_t base, u8 ways)
+{
+	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
+
+	if (!l2ctlr) {
+		dev_err(&pdev->dev, "can not map l2 controller\n");
+		return -EINVAL;
+	}
+
+	/* write bits[0-17] to srbar0 */
+	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
+
+	/* write bits[18-21] to srbare0 */
+#ifdef CONFIG_PHYS_64BIT
+	out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
+#endif
+
+	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
+
+	switch (ways) {
+	case LOCK_WAYS_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
+		break;
+
+	case LOCK_WAYS_TWO_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
+		break;
+
+	case LOCK_WAYS_HALF:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
+		break;
+
+	case LOCK_WAYS_FULL:
+	default:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
+		break;
+	}
+	eieio();
+
+	return 0;
+}
+
+static const struct vm_operations_struct uio_cache_sram_vm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+	.access = generic_access_phys,
+#endif
+};
+
+static int uio_cache_sram_mmap(struct uio_info *info,
+				struct vm_area_struct *vma)
+{
+	struct uio_mem *mem = info->mem;
+
+	if (mem->addr & ~PAGE_MASK)
+		return -ENODEV;
+
+	if ((vma->vm_end - vma->vm_start > mem->size) ||
+		(mem->size == 0) ||
+		(mem->memtype != UIO_MEM_PHYS))
+		return -EINVAL;
+
+	vma->vm_ops = &uio_cache_sram_vm_ops;
+	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
+
+	return remap_pfn_range(vma,
+						   vma->vm_start,
+						   mem->addr >> PAGE_SHIFT,
+						   vma->vm_end - vma->vm_start,
+						   vma->vm_page_prot);
+}
+
+static int uio_cache_sram_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct uio_info *info;
+	struct uio_mem *uiomem;
+	const char *dt_name;
+	phys_addr_t mem_base;
+	u32 l2cache_size;
+	u32 mem_size;
+	u32 rem;
+	u8 ways;
+	int ret;
+
+	if (!node) {
+		dev_err(&pdev->dev, "device's of_node is null\n");
+		return -EINVAL;
+	}
+
+	/* alloc uio_info for one device */
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	/* get optional uio name */
+	if (of_property_read_string(node, "uio_name", &dt_name))
+		dt_name = UIO_NAME;
+
+	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
+	if (!info->name)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-size\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-mem-base", &mem_base);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-base\n");
+		return -EINVAL;
+	}
+
+	if (mem_size == 0) {
+		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing l2cache-size\n");
+		return -EINVAL;
+	}
+
+	rem = l2cache_size % mem_size;
+	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
+	if (rem || (ways & (ways - 1))) {
+		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
+		return -EINVAL;
+	}
+
+	ret = uio_cache_sram_setup(pdev, mem_base, ways);
+	if (ret)
+		return ret;
+
+	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
+		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
+		ret = -ENXIO;
+	}
+
+	info->irq = UIO_IRQ_NONE;
+	info->version = UIO_INFO_VER;
+	info->mmap = uio_cache_sram_mmap;
+	uiomem = info->mem;
+	uiomem->memtype = UIO_MEM_PHYS;
+	uiomem->addr = mem_base;
+	uiomem->size = mem_size;
+	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
+	uiomem->internal_addr = ioremap_coherent(mem_base, mem_size);
+	if (!uiomem->internal_addr) {
+		dev_err(&pdev->dev, "cache ioremep_coherent failed\n");
+		ret = -ENOMEM;
+	}
+
+	/* register uio device */
+	if (uio_register_device(&pdev->dev, info)) {
+		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
+		ret = -ENODEV;
+		goto err_out;
+	}
+
+	platform_set_drvdata(pdev, info);
+	return 0;
+
+err_out:
+	iounmap(info->mem[0].internal_addr);
+	return ret;
+}
+
+static int uio_cache_sram_remove(struct platform_device *pdev)
+{
+	struct uio_info *info = platform_get_drvdata(pdev);
+
+	uio_unregister_device(info);
+	iounmap(info->mem[0].internal_addr);
+
+	return 0;
+}
+
+static const struct of_device_id uio_cache_sram_of_match[] = {
+	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
+
+static struct platform_driver uio_fsl_85xx_cache_sram = {
+	.probe = uio_cache_sram_probe,
+	.remove = uio_cache_sram_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table	= uio_cache_sram_of_match,
+	},
+};
+
+module_platform_driver(uio_fsl_85xx_cache_sram);
+
+MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
+MODULE_DESCRIPTION("Freescale MPC85xx Cache-Sram UIO Platform Driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-09 10:28   ` Wang Wenhu
@ 2022-06-09 13:17     ` Greg KH
  -1 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-09 13:17 UTC (permalink / raw)
  To: Wang Wenhu; +Cc: christophe.leroy, mpe, linuxppc-dev, linux-kernel

On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
> The l2-cache could be optionally configured as SRAM partly or fully.
> Users can make use of it as a block of independent memory that offers
> special usage, such as for debuging or other cratical status info
> storage which keeps consistently even when the whole system crashed.
> 
> The hardware related configuration process utilized the work of the
> earlier implementation, which has been removed now.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
>  drivers/uio/Kconfig                   |  10 +
>  drivers/uio/Makefile                  |   1 +
>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
>  3 files changed, 297 insertions(+)
>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..9199ced03880 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,16 @@ config UIO_NETX
>  	  To compile this driver as a module, choose M here; the module
>  	  will be called uio_netx.
>  
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx Cache-Sram driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Generic driver for accessing the Cache-Sram form user level. This
> +	  is extremely helpful for some user-space applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.

Module name information?

> +
>  config UIO_FSL_ELBC_GPCM
>  	tristate "eLBC/GPCM driver"
>  	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>  obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..d363f9d2b179
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME		"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME		"uio_cache_sram"
> +
> +#define L2CR_L2FI				0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO				0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO			0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL			0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF			0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS		0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART			0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH		0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18		0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4		0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO,
> +	LOCK_WAYS_EIGHTH,
> +	LOCK_WAYS_TWO_EIGHTH,

Why not have values for these?

> +	LOCK_WAYS_HALF = 4,
> +	LOCK_WAYS_FULL = 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	u32	ctl;		/* 0x000 - L2 control */

What is the endian of these u32 values?  You map them directly to
memory, so they must be specified some way, right?  Please make it
obvious what they are.

> +	u8	res1[0xC];
> +	u32	ewar0;		/* 0x010 - External write address 0 */
> +	u32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	u32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	u32	ewar1;		/* 0x020 - External write address 1 */
> +	u32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	u32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	u32	ewar2;		/* 0x030 - External write address 2 */
> +	u32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	u32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	u32	ewar3;		/* 0x040 - External write address 3 */
> +	u32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	u32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	u32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	u32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	u32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	u32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	u32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	u32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	u32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	u32	captdatahi;	/* 0xE20 - Error data high capture */
> +	u32	captdatalo;	/* 0xE24 - Error data low capture */
> +	u32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	u32	errdet;		/* 0xE40 - Error detect */
> +	u32	errdis;		/* 0xE44 - Error disable */
> +	u32	errinten;	/* 0xE48 - Error interrupt enable */
> +	u32	errattr;	/* 0xE4c - Error attribute capture */
> +	u32	erradrrl;	/* 0xE50 - Error address capture low */
> +	u32	erradrrh;	/* 0xE54 - Error address capture high */
> +	u32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};
> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
> +
> +	/* write bits[18-21] to srbare0 */
> +#ifdef CONFIG_PHYS_64BIT

No #ifdef in .c files please.

> +	out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +#endif
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();
> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +#ifdef CONFIG_HAVE_IOREMAP_PROT

Same here.

> +	.access = generic_access_phys,
> +#endif
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +				struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +		(mem->size == 0) ||
> +		(mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +						   vma->vm_start,
> +						   mem->addr >> PAGE_SHIFT,
> +						   vma->vm_end - vma->vm_start,
> +						   vma->vm_page_prot);

Odd indentation, did you use checkpatch.pl on your patch?

> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	if (!node) {
> +		dev_err(&pdev->dev, "device's of_node is null\n");

How can that happen?

> +		return -EINVAL;
> +	}

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-09 13:17     ` Greg KH
  0 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-09 13:17 UTC (permalink / raw)
  To: Wang Wenhu; +Cc: linuxppc-dev, linux-kernel

On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
> The l2-cache could be optionally configured as SRAM partly or fully.
> Users can make use of it as a block of independent memory that offers
> special usage, such as for debuging or other cratical status info
> storage which keeps consistently even when the whole system crashed.
> 
> The hardware related configuration process utilized the work of the
> earlier implementation, which has been removed now.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
>  drivers/uio/Kconfig                   |  10 +
>  drivers/uio/Makefile                  |   1 +
>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
>  3 files changed, 297 insertions(+)
>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..9199ced03880 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,16 @@ config UIO_NETX
>  	  To compile this driver as a module, choose M here; the module
>  	  will be called uio_netx.
>  
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx Cache-Sram driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Generic driver for accessing the Cache-Sram form user level. This
> +	  is extremely helpful for some user-space applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.

Module name information?

> +
>  config UIO_FSL_ELBC_GPCM
>  	tristate "eLBC/GPCM driver"
>  	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>  obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>  obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..d363f9d2b179
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME		"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME		"uio_cache_sram"
> +
> +#define L2CR_L2FI				0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO				0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO			0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL			0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF			0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS		0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART			0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH		0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18		0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4		0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO,
> +	LOCK_WAYS_EIGHTH,
> +	LOCK_WAYS_TWO_EIGHTH,

Why not have values for these?

> +	LOCK_WAYS_HALF = 4,
> +	LOCK_WAYS_FULL = 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	u32	ctl;		/* 0x000 - L2 control */

What is the endian of these u32 values?  You map them directly to
memory, so they must be specified some way, right?  Please make it
obvious what they are.

> +	u8	res1[0xC];
> +	u32	ewar0;		/* 0x010 - External write address 0 */
> +	u32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	u32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	u32	ewar1;		/* 0x020 - External write address 1 */
> +	u32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	u32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	u32	ewar2;		/* 0x030 - External write address 2 */
> +	u32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	u32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	u32	ewar3;		/* 0x040 - External write address 3 */
> +	u32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	u32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	u32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	u32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	u32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	u32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	u32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	u32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	u32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	u32	captdatahi;	/* 0xE20 - Error data high capture */
> +	u32	captdatalo;	/* 0xE24 - Error data low capture */
> +	u32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	u32	errdet;		/* 0xE40 - Error detect */
> +	u32	errdis;		/* 0xE44 - Error disable */
> +	u32	errinten;	/* 0xE48 - Error interrupt enable */
> +	u32	errattr;	/* 0xE4c - Error attribute capture */
> +	u32	erradrrl;	/* 0xE50 - Error address capture low */
> +	u32	erradrrh;	/* 0xE54 - Error address capture high */
> +	u32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};
> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
> +
> +	/* write bits[18-21] to srbare0 */
> +#ifdef CONFIG_PHYS_64BIT

No #ifdef in .c files please.

> +	out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +#endif
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();
> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +#ifdef CONFIG_HAVE_IOREMAP_PROT

Same here.

> +	.access = generic_access_phys,
> +#endif
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +				struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +		(mem->size == 0) ||
> +		(mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +						   vma->vm_start,
> +						   mem->addr >> PAGE_SHIFT,
> +						   vma->vm_end - vma->vm_start,
> +						   vma->vm_page_prot);

Odd indentation, did you use checkpatch.pl on your patch?

> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	if (!node) {
> +		dev_err(&pdev->dev, "device's of_node is null\n");

How can that happen?

> +		return -EINVAL;
> +	}

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-09 13:17     ` Greg KH
@ 2022-06-14  6:09       ` Wenhu Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  6:09 UTC (permalink / raw)
  To: Greg KH; +Cc: christophe.leroy, mpe, linuxppc-dev, linux-kernel

Hi Greg, thanks for the comments.
The questions are replied specifically below.
I have figured out and tested the patch v2, which is to be posted later.
>发件人: Greg KH <gregkh@linuxfoundation.org>
>发送时间: 2022年6月9日 21:17
>收件人: Wang Wenhu <wenhu.wang@hotmail.com>
>抄送: christophe.leroy@csgroup.eu <christophe.leroy@csgroup.eu>; mpe@ellerman.id.au <mpe@ellerman.id.au>; linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
>主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation 
> 
>On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
>> The l2-cache could be optionally configured as SRAM partly or fully.
>> Users can make use of it as a block of independent memory that offers
>> special usage, such as for debuging or other cratical status info
>> storage which keeps consistently even when the whole system crashed.
>> 
>> The hardware related configuration process utilized the work of the
>> earlier implementation, which has been removed now.
>> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
>> 
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
>> ---
>>  drivers/uio/Kconfig                   |  10 +
>>  drivers/uio/Makefile                  |   1 +
>>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
>>  3 files changed, 297 insertions(+)
>>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
>> 
>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> index 2e16c5338e5b..9199ced03880 100644
>> --- a/drivers/uio/Kconfig
>> +++ b/drivers/uio/Kconfig
>> @@ -105,6 +105,16 @@ config UIO_NETX
>>          To compile this driver as a module, choose M here; the module
>>          will be called uio_netx.
>>  
>> +config UIO_FSL_85XX_CACHE_SRAM
>> +     tristate "Freescale 85xx Cache-Sram driver"
>> +     depends on FSL_SOC_BOOKE && PPC32
>> +     help
>> +       Generic driver for accessing the Cache-Sram form user level. This
>> +       is extremely helpful for some user-space applications that require
>> +       high performance memory accesses.
>> +
>> +       If you don't know what to do here, say N.
>
>Module name information?
>
 
More detailed and clearer info in v2

>> +
>>  config UIO_FSL_ELBC_GPCM
>>        tristate "eLBC/GPCM driver"
>>        depends on FSL_LBC
>> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>> index f2f416a14228..1ba07d92a1b1 100644
>> --- a/drivers/uio/Makefile
>> +++ b/drivers/uio/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)      += uio_fsl_elbc_gpcm.o
>>  obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
>>  obj-$(CONFIG_UIO_DFL)        += uio_dfl.o
>> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)        += uio_fsl_85xx_cache_sram.o
>> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> new file mode 100644
>> index 000000000000..d363f9d2b179
>> --- /dev/null
>> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> @@ -0,0 +1,286 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
>> + * All rights reserved.
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/uio_driver.h>
>> +#include <linux/stringify.h>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_address.h>
>> +#include <linux/io.h>
>> +
>> +#define DRIVER_NAME          "uio_mpc85xx_cache_sram"
>> +#define UIO_INFO_VER "0.0.1"
>> +#define UIO_NAME             "uio_cache_sram"
>> +
>> +#define L2CR_L2FI                            0x40000000      /* L2 flash invalidate */
>> +#define L2CR_L2IO                            0x00200000      /* L2 instruction only */
>> +#define L2CR_SRAM_ZERO                       0x00000000      /* L2SRAM zero size */
>> +#define L2CR_SRAM_FULL                       0x00010000      /* L2SRAM full size */
>> +#define L2CR_SRAM_HALF                       0x00020000      /* L2SRAM half size */
>> +#define L2CR_SRAM_TWO_HALFS          0x00030000      /* L2SRAM two half sizes */
>> +#define L2CR_SRAM_QUART                      0x00040000      /* L2SRAM one quarter size */
>> +#define L2CR_SRAM_TWO_QUARTS 0x00050000      /* L2SRAM two quarter size */
>> +#define L2CR_SRAM_EIGHTH             0x00060000      /* L2SRAM one eighth size */
>> +#define L2CR_SRAM_TWO_EIGHTH 0x00070000      /* L2SRAM two eighth size */
>> +
>> +#define L2SRAM_OPTIMAL_SZ_SHIFT      0x00000003      /* Optimum size for L2SRAM */
>> +
>> +#define L2SRAM_BAR_MSK_LO18          0xFFFFC000      /* Lower 18 bits */
>> +#define L2SRAM_BARE_MSK_HI4          0x0000000F      /* Upper 4 bits */
>> +
>> +enum cache_sram_lock_ways {
>> +     LOCK_WAYS_ZERO,
>> +     LOCK_WAYS_EIGHTH,
>> +     LOCK_WAYS_TWO_EIGHTH,
>
>Why not have values for these?
>

full values given in v2

>> +     LOCK_WAYS_HALF = 4,
>> +     LOCK_WAYS_FULL = 8,
>> +};
>> +
>> +struct mpc85xx_l2ctlr {
>> +     u32     ctl;            /* 0x000 - L2 control */
>
>What is the endian of these u32 values?  You map them directly to
>memory, so they must be specified some way, right?  Please make it
>obvious what they are.
>

Surely, the values should be u32 here, modified in v2
The controller info could be found in
"QorIQ™ P2020 Integrated Processor Reference Manual"
"Chapter 6 L2 Look-Aside Cache/SRAM"
See: http://m4udit.dinauz.org/P2020RM_rev0.pdf

>> +     u8      res1[0xC];
>> +     u32     ewar0;          /* 0x010 - External write address 0 */
>> +     u32     ewarea0;        /* 0x014 - External write address extended 0 */
>> +     u32     ewcr0;          /* 0x018 - External write ctrl */
>> +     u8      res2[4];
>> +     u32     ewar1;          /* 0x020 - External write address 1 */
>> +     u32     ewarea1;        /* 0x024 - External write address extended 1 */
>> +     u32     ewcr1;          /* 0x028 - External write ctrl 1 */
>> +     u8      res3[4];
>> +     u32     ewar2;          /* 0x030 - External write address 2 */
>> +     u32     ewarea2;        /* 0x034 - External write address extended 2 */
>> +     u32     ewcr2;          /* 0x038 - External write ctrl 2 */
>> +     u8      res4[4];
>> +     u32     ewar3;          /* 0x040 - External write address 3 */
>> +     u32     ewarea3;        /* 0x044 - External write address extended 3 */
>> +     u32     ewcr3;          /* 0x048 - External write ctrl 3 */
>> +     u8      res5[0xB4];
>> +     u32     srbar0;         /* 0x100 - SRAM base address 0 */
>> +     u32     srbarea0;       /* 0x104 - SRAM base addr reg ext address 0 */
>> +     u32     srbar1;         /* 0x108 - SRAM base address 1 */
>> +     u32     srbarea1;       /* 0x10C - SRAM base addr reg ext address 1 */
>> +     u8      res6[0xCF0];
>> +     u32     errinjhi;       /* 0xE00 - Error injection mask high */
>> +     u32     errinjlo;       /* 0xE04 - Error injection mask low */
>> +     u32     errinjctl;      /* 0xE08 - Error injection tag/ecc control */
>> +     u8      res7[0x14];
>> +     u32     captdatahi;     /* 0xE20 - Error data high capture */
>> +     u32     captdatalo;     /* 0xE24 - Error data low capture */
>> +     u32     captecc;        /* 0xE28 - Error syndrome */
>> +     u8      res8[0x14];
>> +     u32     errdet;         /* 0xE40 - Error detect */
>> +     u32     errdis;         /* 0xE44 - Error disable */
>> +     u32     errinten;       /* 0xE48 - Error interrupt enable */
>> +     u32     errattr;        /* 0xE4c - Error attribute capture */
>> +     u32     erradrrl;       /* 0xE50 - Error address capture low */
>> +     u32     erradrrh;       /* 0xE54 - Error address capture high */
>> +     u32     errctl;         /* 0xE58 - Error control */
>> +     u8      res9[0x1A4];
>> +};
>> +
>> +static int uio_cache_sram_setup(struct platform_device *pdev,
>> +                             phys_addr_t base, u8 ways)
>> +{
>> +     struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
>> +
>> +     if (!l2ctlr) {
>> +             dev_err(&pdev->dev, "can not map l2 controller\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* write bits[0-17] to srbar0 */
>> +     out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
>> +
>> +     /* write bits[18-21] to srbare0 */
>> +#ifdef CONFIG_PHYS_64BIT
>
>No #ifdef in .c files please.
>

#ifdef eliminated in v2

>> +     out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
>> +#endif
>> +
>> +     clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
>> +
>> +     switch (ways) {
>> +     case LOCK_WAYS_EIGHTH:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
>> +             break;
>> +
>> +     case LOCK_WAYS_TWO_EIGHTH:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
>> +             break;
>> +
>> +     case LOCK_WAYS_HALF:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
>> +             break;
>> +
>> +     case LOCK_WAYS_FULL:
>> +     default:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
>> +             break;
>> +     }
>> +     eieio();
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>
>Same here.
>

I tried to eliminate it in mainline
See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
https://lkml.org/lkml/2022/6/10/695

>> +     .access = generic_access_phys,
>> +#endif
>> +};
>> +
>> +static int uio_cache_sram_mmap(struct uio_info *info,
>> +                             struct vm_area_struct *vma)
>> +{
>> +     struct uio_mem *mem = info->mem;
>> +
>> +     if (mem->addr & ~PAGE_MASK)
>> +             return -ENODEV;
>> +
>> +     if ((vma->vm_end - vma->vm_start > mem->size) ||
>> +             (mem->size == 0) ||
>> +             (mem->memtype != UIO_MEM_PHYS))
>> +             return -EINVAL;
>> +
>> +     vma->vm_ops = &uio_cache_sram_vm_ops;
>> +     vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
>> +
>> +     return remap_pfn_range(vma,
>> +                                                vma->vm_start,
>> +                                                mem->addr >> PAGE_SHIFT,
>> +                                                vma->vm_end - vma->vm_start,
>> +                                                vma->vm_page_prot);
>
>Odd indentation, did you use checkpatch.pl on your patch?
>

Actually, I checked with the scripts, and there was no warning here.
I also checked in text editors and vim, if I translate tab with 4 spaces,
the "vma/mem" areas in the 5 lines were aligned.

>> +}
>> +
>> +static int uio_cache_sram_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *node = pdev->dev.of_node;
>> +     struct uio_info *info;
>> +     struct uio_mem *uiomem;
>> +     const char *dt_name;
>> +     phys_addr_t mem_base;
>> +     u32 l2cache_size;
>> +     u32 mem_size;
>> +     u32 rem;
>> +     u8 ways;
>> +     int ret;
>> +
>> +     if (!node) {
>> +             dev_err(&pdev->dev, "device's of_node is null\n");
>
>How can that happen?
>

Redundant and will be removed in v2

>> +             return -EINVAL;
>> +     }
>
>thanks,
>
>greg k-h
>

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  6:09       ` Wenhu Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  6:09 UTC (permalink / raw)
  To: Greg KH; +Cc: linuxppc-dev, linux-kernel

Hi Greg, thanks for the comments.
The questions are replied specifically below.
I have figured out and tested the patch v2, which is to be posted later.
>发件人: Greg KH <gregkh@linuxfoundation.org>
>发送时间: 2022年6月9日 21:17
>收件人: Wang Wenhu <wenhu.wang@hotmail.com>
>抄送: christophe.leroy@csgroup.eu <christophe.leroy@csgroup.eu>; mpe@ellerman.id.au <mpe@ellerman.id.au>; linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
>主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation 
> 
>On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
>> The l2-cache could be optionally configured as SRAM partly or fully.
>> Users can make use of it as a block of independent memory that offers
>> special usage, such as for debuging or other cratical status info
>> storage which keeps consistently even when the whole system crashed.
>> 
>> The hardware related configuration process utilized the work of the
>> earlier implementation, which has been removed now.
>> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
>> 
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
>> ---
>>  drivers/uio/Kconfig                   |  10 +
>>  drivers/uio/Makefile                  |   1 +
>>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
>>  3 files changed, 297 insertions(+)
>>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
>> 
>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> index 2e16c5338e5b..9199ced03880 100644
>> --- a/drivers/uio/Kconfig
>> +++ b/drivers/uio/Kconfig
>> @@ -105,6 +105,16 @@ config UIO_NETX
>>          To compile this driver as a module, choose M here; the module
>>          will be called uio_netx.
>>  
>> +config UIO_FSL_85XX_CACHE_SRAM
>> +     tristate "Freescale 85xx Cache-Sram driver"
>> +     depends on FSL_SOC_BOOKE && PPC32
>> +     help
>> +       Generic driver for accessing the Cache-Sram form user level. This
>> +       is extremely helpful for some user-space applications that require
>> +       high performance memory accesses.
>> +
>> +       If you don't know what to do here, say N.
>
>Module name information?
>
 
More detailed and clearer info in v2

>> +
>>  config UIO_FSL_ELBC_GPCM
>>        tristate "eLBC/GPCM driver"
>>        depends on FSL_LBC
>> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>> index f2f416a14228..1ba07d92a1b1 100644
>> --- a/drivers/uio/Makefile
>> +++ b/drivers/uio/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)      += uio_fsl_elbc_gpcm.o
>>  obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
>>  obj-$(CONFIG_UIO_DFL)        += uio_dfl.o
>> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)        += uio_fsl_85xx_cache_sram.o
>> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> new file mode 100644
>> index 000000000000..d363f9d2b179
>> --- /dev/null
>> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> @@ -0,0 +1,286 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
>> + * All rights reserved.
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/uio_driver.h>
>> +#include <linux/stringify.h>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_address.h>
>> +#include <linux/io.h>
>> +
>> +#define DRIVER_NAME          "uio_mpc85xx_cache_sram"
>> +#define UIO_INFO_VER "0.0.1"
>> +#define UIO_NAME             "uio_cache_sram"
>> +
>> +#define L2CR_L2FI                            0x40000000      /* L2 flash invalidate */
>> +#define L2CR_L2IO                            0x00200000      /* L2 instruction only */
>> +#define L2CR_SRAM_ZERO                       0x00000000      /* L2SRAM zero size */
>> +#define L2CR_SRAM_FULL                       0x00010000      /* L2SRAM full size */
>> +#define L2CR_SRAM_HALF                       0x00020000      /* L2SRAM half size */
>> +#define L2CR_SRAM_TWO_HALFS          0x00030000      /* L2SRAM two half sizes */
>> +#define L2CR_SRAM_QUART                      0x00040000      /* L2SRAM one quarter size */
>> +#define L2CR_SRAM_TWO_QUARTS 0x00050000      /* L2SRAM two quarter size */
>> +#define L2CR_SRAM_EIGHTH             0x00060000      /* L2SRAM one eighth size */
>> +#define L2CR_SRAM_TWO_EIGHTH 0x00070000      /* L2SRAM two eighth size */
>> +
>> +#define L2SRAM_OPTIMAL_SZ_SHIFT      0x00000003      /* Optimum size for L2SRAM */
>> +
>> +#define L2SRAM_BAR_MSK_LO18          0xFFFFC000      /* Lower 18 bits */
>> +#define L2SRAM_BARE_MSK_HI4          0x0000000F      /* Upper 4 bits */
>> +
>> +enum cache_sram_lock_ways {
>> +     LOCK_WAYS_ZERO,
>> +     LOCK_WAYS_EIGHTH,
>> +     LOCK_WAYS_TWO_EIGHTH,
>
>Why not have values for these?
>

full values given in v2

>> +     LOCK_WAYS_HALF = 4,
>> +     LOCK_WAYS_FULL = 8,
>> +};
>> +
>> +struct mpc85xx_l2ctlr {
>> +     u32     ctl;            /* 0x000 - L2 control */
>
>What is the endian of these u32 values?  You map them directly to
>memory, so they must be specified some way, right?  Please make it
>obvious what they are.
>

Surely, the values should be u32 here, modified in v2
The controller info could be found in
"QorIQ™ P2020 Integrated Processor Reference Manual"
"Chapter 6 L2 Look-Aside Cache/SRAM"
See: http://m4udit.dinauz.org/P2020RM_rev0.pdf

>> +     u8      res1[0xC];
>> +     u32     ewar0;          /* 0x010 - External write address 0 */
>> +     u32     ewarea0;        /* 0x014 - External write address extended 0 */
>> +     u32     ewcr0;          /* 0x018 - External write ctrl */
>> +     u8      res2[4];
>> +     u32     ewar1;          /* 0x020 - External write address 1 */
>> +     u32     ewarea1;        /* 0x024 - External write address extended 1 */
>> +     u32     ewcr1;          /* 0x028 - External write ctrl 1 */
>> +     u8      res3[4];
>> +     u32     ewar2;          /* 0x030 - External write address 2 */
>> +     u32     ewarea2;        /* 0x034 - External write address extended 2 */
>> +     u32     ewcr2;          /* 0x038 - External write ctrl 2 */
>> +     u8      res4[4];
>> +     u32     ewar3;          /* 0x040 - External write address 3 */
>> +     u32     ewarea3;        /* 0x044 - External write address extended 3 */
>> +     u32     ewcr3;          /* 0x048 - External write ctrl 3 */
>> +     u8      res5[0xB4];
>> +     u32     srbar0;         /* 0x100 - SRAM base address 0 */
>> +     u32     srbarea0;       /* 0x104 - SRAM base addr reg ext address 0 */
>> +     u32     srbar1;         /* 0x108 - SRAM base address 1 */
>> +     u32     srbarea1;       /* 0x10C - SRAM base addr reg ext address 1 */
>> +     u8      res6[0xCF0];
>> +     u32     errinjhi;       /* 0xE00 - Error injection mask high */
>> +     u32     errinjlo;       /* 0xE04 - Error injection mask low */
>> +     u32     errinjctl;      /* 0xE08 - Error injection tag/ecc control */
>> +     u8      res7[0x14];
>> +     u32     captdatahi;     /* 0xE20 - Error data high capture */
>> +     u32     captdatalo;     /* 0xE24 - Error data low capture */
>> +     u32     captecc;        /* 0xE28 - Error syndrome */
>> +     u8      res8[0x14];
>> +     u32     errdet;         /* 0xE40 - Error detect */
>> +     u32     errdis;         /* 0xE44 - Error disable */
>> +     u32     errinten;       /* 0xE48 - Error interrupt enable */
>> +     u32     errattr;        /* 0xE4c - Error attribute capture */
>> +     u32     erradrrl;       /* 0xE50 - Error address capture low */
>> +     u32     erradrrh;       /* 0xE54 - Error address capture high */
>> +     u32     errctl;         /* 0xE58 - Error control */
>> +     u8      res9[0x1A4];
>> +};
>> +
>> +static int uio_cache_sram_setup(struct platform_device *pdev,
>> +                             phys_addr_t base, u8 ways)
>> +{
>> +     struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
>> +
>> +     if (!l2ctlr) {
>> +             dev_err(&pdev->dev, "can not map l2 controller\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* write bits[0-17] to srbar0 */
>> +     out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
>> +
>> +     /* write bits[18-21] to srbare0 */
>> +#ifdef CONFIG_PHYS_64BIT
>
>No #ifdef in .c files please.
>

#ifdef eliminated in v2

>> +     out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
>> +#endif
>> +
>> +     clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
>> +
>> +     switch (ways) {
>> +     case LOCK_WAYS_EIGHTH:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
>> +             break;
>> +
>> +     case LOCK_WAYS_TWO_EIGHTH:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
>> +             break;
>> +
>> +     case LOCK_WAYS_HALF:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
>> +             break;
>> +
>> +     case LOCK_WAYS_FULL:
>> +     default:
>> +             setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
>> +             break;
>> +     }
>> +     eieio();
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>
>Same here.
>

I tried to eliminate it in mainline
See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
https://lkml.org/lkml/2022/6/10/695

>> +     .access = generic_access_phys,
>> +#endif
>> +};
>> +
>> +static int uio_cache_sram_mmap(struct uio_info *info,
>> +                             struct vm_area_struct *vma)
>> +{
>> +     struct uio_mem *mem = info->mem;
>> +
>> +     if (mem->addr & ~PAGE_MASK)
>> +             return -ENODEV;
>> +
>> +     if ((vma->vm_end - vma->vm_start > mem->size) ||
>> +             (mem->size == 0) ||
>> +             (mem->memtype != UIO_MEM_PHYS))
>> +             return -EINVAL;
>> +
>> +     vma->vm_ops = &uio_cache_sram_vm_ops;
>> +     vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
>> +
>> +     return remap_pfn_range(vma,
>> +                                                vma->vm_start,
>> +                                                mem->addr >> PAGE_SHIFT,
>> +                                                vma->vm_end - vma->vm_start,
>> +                                                vma->vm_page_prot);
>
>Odd indentation, did you use checkpatch.pl on your patch?
>

Actually, I checked with the scripts, and there was no warning here.
I also checked in text editors and vim, if I translate tab with 4 spaces,
the "vma/mem" areas in the 5 lines were aligned.

>> +}
>> +
>> +static int uio_cache_sram_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *node = pdev->dev.of_node;
>> +     struct uio_info *info;
>> +     struct uio_mem *uiomem;
>> +     const char *dt_name;
>> +     phys_addr_t mem_base;
>> +     u32 l2cache_size;
>> +     u32 mem_size;
>> +     u32 rem;
>> +     u8 ways;
>> +     int ret;
>> +
>> +     if (!node) {
>> +             dev_err(&pdev->dev, "device's of_node is null\n");
>
>How can that happen?
>

Redundant and will be removed in v2

>> +             return -EINVAL;
>> +     }
>
>thanks,
>
>greg k-h
>

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  6:09       ` Wenhu Wang
@ 2022-06-14  6:34         ` Greg KH
  -1 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-14  6:34 UTC (permalink / raw)
  To: Wenhu Wang; +Cc: christophe.leroy, mpe, linuxppc-dev, linux-kernel

On Tue, Jun 14, 2022 at 06:09:35AM +0000, Wenhu Wang wrote:
> Hi Greg, thanks for the comments.
> The questions are replied specifically below.
> I have figured out and tested the patch v2, which is to be posted later.
> >发件人: Greg KH <gregkh@linuxfoundation.org>
> >发送时间: 2022年6月9日 21:17
> >收件人: Wang Wenhu <wenhu.wang@hotmail.com>
> >抄送: christophe.leroy@csgroup.eu <christophe.leroy@csgroup.eu>; mpe@ellerman.id.au <mpe@ellerman.id.au>; linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> >主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation 
> > 
> >On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
> >> The l2-cache could be optionally configured as SRAM partly or fully.
> >> Users can make use of it as a block of independent memory that offers
> >> special usage, such as for debuging or other cratical status info
> >> storage which keeps consistently even when the whole system crashed.
> >> 
> >> The hardware related configuration process utilized the work of the
> >> earlier implementation, which has been removed now.
> >> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> >> 
> >> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> >> ---
> >>  drivers/uio/Kconfig                   |  10 +
> >>  drivers/uio/Makefile                  |   1 +
> >>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
> >>  3 files changed, 297 insertions(+)
> >>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> >> 
> >> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> >> index 2e16c5338e5b..9199ced03880 100644
> >> --- a/drivers/uio/Kconfig
> >> +++ b/drivers/uio/Kconfig
> >> @@ -105,6 +105,16 @@ config UIO_NETX
> >>          To compile this driver as a module, choose M here; the module
> >>          will be called uio_netx.
> >>  
> >> +config UIO_FSL_85XX_CACHE_SRAM
> >> +     tristate "Freescale 85xx Cache-Sram driver"
> >> +     depends on FSL_SOC_BOOKE && PPC32
> >> +     help
> >> +       Generic driver for accessing the Cache-Sram form user level. This
> >> +       is extremely helpful for some user-space applications that require
> >> +       high performance memory accesses.
> >> +
> >> +       If you don't know what to do here, say N.
> >
> >Module name information?
> >
>  
> More detailed and clearer info in v2
> 
> >> +
> >>  config UIO_FSL_ELBC_GPCM
> >>        tristate "eLBC/GPCM driver"
> >>        depends on FSL_LBC
> >> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> >> index f2f416a14228..1ba07d92a1b1 100644
> >> --- a/drivers/uio/Makefile
> >> +++ b/drivers/uio/Makefile
> >> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
> >>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)      += uio_fsl_elbc_gpcm.o
> >>  obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
> >>  obj-$(CONFIG_UIO_DFL)        += uio_dfl.o
> >> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)        += uio_fsl_85xx_cache_sram.o
> >> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> new file mode 100644
> >> index 000000000000..d363f9d2b179
> >> --- /dev/null
> >> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> @@ -0,0 +1,286 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> >> + * All rights reserved.
> >> + */
> >> +
> >> +#include <linux/platform_device.h>
> >> +#include <linux/uio_driver.h>
> >> +#include <linux/stringify.h>
> >> +#include <linux/module.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/io.h>
> >> +
> >> +#define DRIVER_NAME          "uio_mpc85xx_cache_sram"
> >> +#define UIO_INFO_VER "0.0.1"
> >> +#define UIO_NAME             "uio_cache_sram"
> >> +
> >> +#define L2CR_L2FI                            0x40000000      /* L2 flash invalidate */
> >> +#define L2CR_L2IO                            0x00200000      /* L2 instruction only */
> >> +#define L2CR_SRAM_ZERO                       0x00000000      /* L2SRAM zero size */
> >> +#define L2CR_SRAM_FULL                       0x00010000      /* L2SRAM full size */
> >> +#define L2CR_SRAM_HALF                       0x00020000      /* L2SRAM half size */
> >> +#define L2CR_SRAM_TWO_HALFS          0x00030000      /* L2SRAM two half sizes */
> >> +#define L2CR_SRAM_QUART                      0x00040000      /* L2SRAM one quarter size */
> >> +#define L2CR_SRAM_TWO_QUARTS 0x00050000      /* L2SRAM two quarter size */
> >> +#define L2CR_SRAM_EIGHTH             0x00060000      /* L2SRAM one eighth size */
> >> +#define L2CR_SRAM_TWO_EIGHTH 0x00070000      /* L2SRAM two eighth size */
> >> +
> >> +#define L2SRAM_OPTIMAL_SZ_SHIFT      0x00000003      /* Optimum size for L2SRAM */
> >> +
> >> +#define L2SRAM_BAR_MSK_LO18          0xFFFFC000      /* Lower 18 bits */
> >> +#define L2SRAM_BARE_MSK_HI4          0x0000000F      /* Upper 4 bits */
> >> +
> >> +enum cache_sram_lock_ways {
> >> +     LOCK_WAYS_ZERO,
> >> +     LOCK_WAYS_EIGHTH,
> >> +     LOCK_WAYS_TWO_EIGHTH,
> >
> >Why not have values for these?
> >
> 
> full values given in v2
> 
> >> +     LOCK_WAYS_HALF = 4,
> >> +     LOCK_WAYS_FULL = 8,
> >> +};
> >> +
> >> +struct mpc85xx_l2ctlr {
> >> +     u32     ctl;            /* 0x000 - L2 control */
> >
> >What is the endian of these u32 values?  You map them directly to
> >memory, so they must be specified some way, right?  Please make it
> >obvious what they are.
> >
> 
> Surely, the values should be u32 here, modified in v2
> The controller info could be found in
> "QorIQ™ P2020 Integrated Processor Reference Manual"
> "Chapter 6 L2 Look-Aside Cache/SRAM"
> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf

That's not the answer to my question :)

These are big-endian, right?  Please mark them as such and access them
properly with the correct functions.

> >> +     return remap_pfn_range(vma,
> >> +                                                vma->vm_start,
> >> +                                                mem->addr >> PAGE_SHIFT,
> >> +                                                vma->vm_end - vma->vm_start,
> >> +                                                vma->vm_page_prot);
> >
> >Odd indentation, did you use checkpatch.pl on your patch?
> >
> 
> Actually, I checked with the scripts, and there was no warning here.
> I also checked in text editors and vim, if I translate tab with 4 spaces,
> the "vma/mem" areas in the 5 lines were aligned.

Tabs in Linux are always 8 spaces wide.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  6:34         ` Greg KH
  0 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-14  6:34 UTC (permalink / raw)
  To: Wenhu Wang; +Cc: linuxppc-dev, linux-kernel

On Tue, Jun 14, 2022 at 06:09:35AM +0000, Wenhu Wang wrote:
> Hi Greg, thanks for the comments.
> The questions are replied specifically below.
> I have figured out and tested the patch v2, which is to be posted later.
> >发件人: Greg KH <gregkh@linuxfoundation.org>
> >发送时间: 2022年6月9日 21:17
> >收件人: Wang Wenhu <wenhu.wang@hotmail.com>
> >抄送: christophe.leroy@csgroup.eu <christophe.leroy@csgroup.eu>; mpe@ellerman.id.au <mpe@ellerman.id.au>; linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> >主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation 
> > 
> >On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
> >> The l2-cache could be optionally configured as SRAM partly or fully.
> >> Users can make use of it as a block of independent memory that offers
> >> special usage, such as for debuging or other cratical status info
> >> storage which keeps consistently even when the whole system crashed.
> >> 
> >> The hardware related configuration process utilized the work of the
> >> earlier implementation, which has been removed now.
> >> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> >> 
> >> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> >> ---
> >>  drivers/uio/Kconfig                   |  10 +
> >>  drivers/uio/Makefile                  |   1 +
> >>  drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++++++++++++++++++++++++++
> >>  3 files changed, 297 insertions(+)
> >>  create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> >> 
> >> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> >> index 2e16c5338e5b..9199ced03880 100644
> >> --- a/drivers/uio/Kconfig
> >> +++ b/drivers/uio/Kconfig
> >> @@ -105,6 +105,16 @@ config UIO_NETX
> >>          To compile this driver as a module, choose M here; the module
> >>          will be called uio_netx.
> >>  
> >> +config UIO_FSL_85XX_CACHE_SRAM
> >> +     tristate "Freescale 85xx Cache-Sram driver"
> >> +     depends on FSL_SOC_BOOKE && PPC32
> >> +     help
> >> +       Generic driver for accessing the Cache-Sram form user level. This
> >> +       is extremely helpful for some user-space applications that require
> >> +       high performance memory accesses.
> >> +
> >> +       If you don't know what to do here, say N.
> >
> >Module name information?
> >
>  
> More detailed and clearer info in v2
> 
> >> +
> >>  config UIO_FSL_ELBC_GPCM
> >>        tristate "eLBC/GPCM driver"
> >>        depends on FSL_LBC
> >> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> >> index f2f416a14228..1ba07d92a1b1 100644
> >> --- a/drivers/uio/Makefile
> >> +++ b/drivers/uio/Makefile
> >> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
> >>  obj-$(CONFIG_UIO_FSL_ELBC_GPCM)      += uio_fsl_elbc_gpcm.o
> >>  obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
> >>  obj-$(CONFIG_UIO_DFL)        += uio_dfl.o
> >> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)        += uio_fsl_85xx_cache_sram.o
> >> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> new file mode 100644
> >> index 000000000000..d363f9d2b179
> >> --- /dev/null
> >> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> @@ -0,0 +1,286 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> >> + * All rights reserved.
> >> + */
> >> +
> >> +#include <linux/platform_device.h>
> >> +#include <linux/uio_driver.h>
> >> +#include <linux/stringify.h>
> >> +#include <linux/module.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/io.h>
> >> +
> >> +#define DRIVER_NAME          "uio_mpc85xx_cache_sram"
> >> +#define UIO_INFO_VER "0.0.1"
> >> +#define UIO_NAME             "uio_cache_sram"
> >> +
> >> +#define L2CR_L2FI                            0x40000000      /* L2 flash invalidate */
> >> +#define L2CR_L2IO                            0x00200000      /* L2 instruction only */
> >> +#define L2CR_SRAM_ZERO                       0x00000000      /* L2SRAM zero size */
> >> +#define L2CR_SRAM_FULL                       0x00010000      /* L2SRAM full size */
> >> +#define L2CR_SRAM_HALF                       0x00020000      /* L2SRAM half size */
> >> +#define L2CR_SRAM_TWO_HALFS          0x00030000      /* L2SRAM two half sizes */
> >> +#define L2CR_SRAM_QUART                      0x00040000      /* L2SRAM one quarter size */
> >> +#define L2CR_SRAM_TWO_QUARTS 0x00050000      /* L2SRAM two quarter size */
> >> +#define L2CR_SRAM_EIGHTH             0x00060000      /* L2SRAM one eighth size */
> >> +#define L2CR_SRAM_TWO_EIGHTH 0x00070000      /* L2SRAM two eighth size */
> >> +
> >> +#define L2SRAM_OPTIMAL_SZ_SHIFT      0x00000003      /* Optimum size for L2SRAM */
> >> +
> >> +#define L2SRAM_BAR_MSK_LO18          0xFFFFC000      /* Lower 18 bits */
> >> +#define L2SRAM_BARE_MSK_HI4          0x0000000F      /* Upper 4 bits */
> >> +
> >> +enum cache_sram_lock_ways {
> >> +     LOCK_WAYS_ZERO,
> >> +     LOCK_WAYS_EIGHTH,
> >> +     LOCK_WAYS_TWO_EIGHTH,
> >
> >Why not have values for these?
> >
> 
> full values given in v2
> 
> >> +     LOCK_WAYS_HALF = 4,
> >> +     LOCK_WAYS_FULL = 8,
> >> +};
> >> +
> >> +struct mpc85xx_l2ctlr {
> >> +     u32     ctl;            /* 0x000 - L2 control */
> >
> >What is the endian of these u32 values?  You map them directly to
> >memory, so they must be specified some way, right?  Please make it
> >obvious what they are.
> >
> 
> Surely, the values should be u32 here, modified in v2
> The controller info could be found in
> "QorIQ™ P2020 Integrated Processor Reference Manual"
> "Chapter 6 L2 Look-Aside Cache/SRAM"
> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf

That's not the answer to my question :)

These are big-endian, right?  Please mark them as such and access them
properly with the correct functions.

> >> +     return remap_pfn_range(vma,
> >> +                                                vma->vm_start,
> >> +                                                mem->addr >> PAGE_SHIFT,
> >> +                                                vma->vm_end - vma->vm_start,
> >> +                                                vma->vm_page_prot);
> >
> >Odd indentation, did you use checkpatch.pl on your patch?
> >
> 
> Actually, I checked with the scripts, and there was no warning here.
> I also checked in text editors and vim, if I translate tab with 4 spaces,
> the "vma/mem" areas in the 5 lines were aligned.

Tabs in Linux are always 8 spaces wide.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  6:34         ` Greg KH
@ 2022-06-14  7:12           ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:12 UTC (permalink / raw)
  To: Greg KH, Wenhu Wang; +Cc: mpe, linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:34, Greg KH a écrit :
> On Tue, Jun 14, 2022 at 06:09:35AM +0000, Wenhu Wang wrote:
>>>
>>> Odd indentation, did you use checkpatch.pl on your patch?
>>>
>>
>> Actually, I checked with the scripts, and there was no warning here.
>> I also checked in text editors and vim, if I translate tab with 4 spaces,
>> the "vma/mem" areas in the 5 lines were aligned.
> 
> Tabs in Linux are always 8 spaces wide.
> 

See 
https://docs.kernel.org/process/coding-style.html?highlight=coding+style#indentation

Tabs are 8 characters, and thus indentations are also 8 characters. 
There are heretic movements that try to make indentations 4 (or even 2!) 
characters deep, and that is akin to trying to define the value of PI to 
be 3.

Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  7:12           ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:12 UTC (permalink / raw)
  To: Greg KH, Wenhu Wang; +Cc: linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:34, Greg KH a écrit :
> On Tue, Jun 14, 2022 at 06:09:35AM +0000, Wenhu Wang wrote:
>>>
>>> Odd indentation, did you use checkpatch.pl on your patch?
>>>
>>
>> Actually, I checked with the scripts, and there was no warning here.
>> I also checked in text editors and vim, if I translate tab with 4 spaces,
>> the "vma/mem" areas in the 5 lines were aligned.
> 
> Tabs in Linux are always 8 spaces wide.
> 

See 
https://docs.kernel.org/process/coding-style.html?highlight=coding+style#indentation

Tabs are 8 characters, and thus indentations are also 8 characters. 
There are heretic movements that try to make indentations 4 (or even 2!) 
characters deep, and that is akin to trying to define the value of PI to 
be 3.

Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  6:09       ` Wenhu Wang
@ 2022-06-14  7:18         ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:18 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>
>> Same here.
>>
> 
> I tried to eliminate it in mainline
> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
> https://lkml.org/lkml/2022/6/10/695
> 

I looked at that patch.

I don't think you can just drop the #ifdef in function 
__access_remote_vm() in mm/memory.c

You have to replace it with something like:

	if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
		break;

Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  7:18         ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:18 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>
>> Same here.
>>
> 
> I tried to eliminate it in mainline
> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
> https://lkml.org/lkml/2022/6/10/695
> 

I looked at that patch.

I don't think you can just drop the #ifdef in function 
__access_remote_vm() in mm/memory.c

You have to replace it with something like:

	if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
		break;

Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  7:18         ` Christophe Leroy
@ 2022-06-14  7:26           ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:26 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel



Le 14/06/2022 à 09:18, Christophe Leroy a écrit :
> 
> 
> Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>>
>>> Same here.
>>>
>>
>> I tried to eliminate it in mainline
>> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
>> https://lkml.org/lkml/2022/6/10/695
>>
> 
> I looked at that patch.
> 
> I don't think you can just drop the #ifdef in function 
> __access_remote_vm() in mm/memory.c
> 
> You have to replace it with something like:
> 
>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>          break;
> 


Another thing in that patch:

By making generic_access_phys() a static inline, it means that everytime 
you refer to the address of that function in a vm_operations_struct 
struct, the compiler has to provide an outlined instance of the 
function. It means you'll likely have several instances of a 
generic_access_phys().

What you could do instead is to add the following at the start of 
generic_access_phys() in mm/memory.c :

	if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
		return 0;

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  7:26           ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14  7:26 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: linuxppc-dev, linux-kernel



Le 14/06/2022 à 09:18, Christophe Leroy a écrit :
> 
> 
> Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>>
>>> Same here.
>>>
>>
>> I tried to eliminate it in mainline
>> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
>> https://lkml.org/lkml/2022/6/10/695
>>
> 
> I looked at that patch.
> 
> I don't think you can just drop the #ifdef in function 
> __access_remote_vm() in mm/memory.c
> 
> You have to replace it with something like:
> 
>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>          break;
> 


Another thing in that patch:

By making generic_access_phys() a static inline, it means that everytime 
you refer to the address of that function in a vm_operations_struct 
struct, the compiler has to provide an outlined instance of the 
function. It means you'll likely have several instances of a 
generic_access_phys().

What you could do instead is to add the following at the start of 
generic_access_phys() in mm/memory.c :

	if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
		return 0;

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  7:26           ` Christophe Leroy
@ 2022-06-14  7:53             ` Wenhu Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  7:53 UTC (permalink / raw)
  To: Christophe Leroy, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel

>> >> +
>> >> +struct mpc85xx_l2ctlr {
>> >> +     u32     ctl;            /* 0x000 - L2 control */
>> >
>> >What is the endian of these u32 values?  You map them directly to
>> >memory, so they must be specified some way, right?  Please make it
>> >obvious what they are.
>> >
>>
>> Surely, the values should be u32 here, modified in v2
>> The controller info could be found in
>> "QorIQ™ P2020 Integrated Processor Reference Manual"
>> "Chapter 6 L2 Look-Aside Cache/SRAM"
>> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
>
>That's not the answer to my question :)
>
>These are big-endian, right?  Please mark them as such and access them
>properly with the correct functions.

Yes, they are big-edian.
Does it work to add comments(about order and access functions) for the structure ahead of it?
And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {……};

>Tabs in Linux are always 8 spaces wide.
>

I will re-confirm the v2 of identation.

>>
>> I looked at that patch.
>>
>> I don't think you can just drop the #ifdef in function
>> __access_remote_vm() in mm/memory.c
>>
>> You have to replace it with something like:
>>
>>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>          break;
>>
>
>
>Another thing in that patch:
>
>By making generic_access_phys() a static inline, it means that everytime
>you refer to the address of that function in a vm_operations_struct
>struct, the compiler has to provide an outlined instance of the
>function. It means you'll likely have several instances of a
>generic_access_phys().
>
>What you could do instead is to add the following at the start of
>generic_access_phys() in mm/memory.c :
>
>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>                return 0;
>

It is really a better chmoce, thanks for the advice.
Multiple instances exist as you mentioned, the block returns 0 with no-op
instance which makes no difference with the function return value.

I will update the patch after a re-confirming.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  7:53             ` Wenhu Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  7:53 UTC (permalink / raw)
  To: Christophe Leroy, Greg KH; +Cc: linuxppc-dev, linux-kernel

>> >> +
>> >> +struct mpc85xx_l2ctlr {
>> >> +     u32     ctl;            /* 0x000 - L2 control */
>> >
>> >What is the endian of these u32 values?  You map them directly to
>> >memory, so they must be specified some way, right?  Please make it
>> >obvious what they are.
>> >
>>
>> Surely, the values should be u32 here, modified in v2
>> The controller info could be found in
>> "QorIQ™ P2020 Integrated Processor Reference Manual"
>> "Chapter 6 L2 Look-Aside Cache/SRAM"
>> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
>
>That's not the answer to my question :)
>
>These are big-endian, right?  Please mark them as such and access them
>properly with the correct functions.

Yes, they are big-edian.
Does it work to add comments(about order and access functions) for the structure ahead of it?
And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {……};

>Tabs in Linux are always 8 spaces wide.
>

I will re-confirm the v2 of identation.

>>
>> I looked at that patch.
>>
>> I don't think you can just drop the #ifdef in function
>> __access_remote_vm() in mm/memory.c
>>
>> You have to replace it with something like:
>>
>>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>          break;
>>
>
>
>Another thing in that patch:
>
>By making generic_access_phys() a static inline, it means that everytime
>you refer to the address of that function in a vm_operations_struct
>struct, the compiler has to provide an outlined instance of the
>function. It means you'll likely have several instances of a
>generic_access_phys().
>
>What you could do instead is to add the following at the start of
>generic_access_phys() in mm/memory.c :
>
>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>                return 0;
>

It is really a better chmoce, thanks for the advice.
Multiple instances exist as you mentioned, the block returns 0 with no-op
instance which makes no difference with the function return value.

I will update the patch after a re-confirming.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  7:53             ` Wenhu Wang
@ 2022-06-14  8:03               ` Greg KH
  -1 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-14  8:03 UTC (permalink / raw)
  To: Wenhu Wang; +Cc: Christophe Leroy, mpe, linuxppc-dev, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1080 bytes --]

On Tue, Jun 14, 2022 at 07:53:46AM +0000, Wenhu Wang wrote:
> >> >> +
> >> >> +struct mpc85xx_l2ctlr {
> >> >> +     u32     ctl;            /* 0x000 - L2 control */
> >> >
> >> >What is the endian of these u32 values?  You map them directly to
> >> >memory, so they must be specified some way, right?  Please make it
> >> >obvious what they are.
> >> >
> >>
> >> Surely, the values should be u32 here, modified in v2
> >> The controller info could be found in
> >> "QorIQ¢â P2020 Integrated Processor Reference Manual"
> >> "Chapter 6 L2 Look-Aside Cache/SRAM"
> >> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
> >
> >That's not the answer to my question :)
> >
> >These are big-endian, right?  Please mark them as such and access them
> >properly with the correct functions.
> 
> Yes, they are big-edian.
> Does it work to add comments(about order and access functions) for the structure ahead of it£¿
> And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {¡¦¡¦};

No, not comments, these should be of the type __be32, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  8:03               ` Greg KH
  0 siblings, 0 replies; 52+ messages in thread
From: Greg KH @ 2022-06-14  8:03 UTC (permalink / raw)
  To: Wenhu Wang; +Cc: linuxppc-dev, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1080 bytes --]

On Tue, Jun 14, 2022 at 07:53:46AM +0000, Wenhu Wang wrote:
> >> >> +
> >> >> +struct mpc85xx_l2ctlr {
> >> >> +     u32     ctl;            /* 0x000 - L2 control */
> >> >
> >> >What is the endian of these u32 values?  You map them directly to
> >> >memory, so they must be specified some way, right?  Please make it
> >> >obvious what they are.
> >> >
> >>
> >> Surely, the values should be u32 here, modified in v2
> >> The controller info could be found in
> >> "QorIQ¢â P2020 Integrated Processor Reference Manual"
> >> "Chapter 6 L2 Look-Aside Cache/SRAM"
> >> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
> >
> >That's not the answer to my question :)
> >
> >These are big-endian, right?  Please mark them as such and access them
> >properly with the correct functions.
> 
> Yes, they are big-edian.
> Does it work to add comments(about order and access functions) for the structure ahead of it£¿
> And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {¡¦¡¦};

No, not comments, these should be of the type __be32, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  8:03               ` Greg KH
@ 2022-06-14  8:39                 ` Wenhu Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  8:39 UTC (permalink / raw)
  To: Greg KH; +Cc: Christophe Leroy, mpe, linuxppc-dev, linux-kernel

>On Tue, Jun 14, 2022 at 07:53:46AM +0000, Wenhu Wang wrote:
>> >> >> +
>> >> >> +struct mpc85xx_l2ctlr {
>> >> >> +     u32     ctl;            /* 0x000 - L2 control */
>> >> >
>> >> >What is the endian of these u32 values?  You map them directly to
>> >> >memory, so they must be specified some way, right?  Please make it
>> >> >obvious what they are.
>> >> >
>> >>
>> >> Surely, the values should be u32 here, modified in v2
>> >> The controller info could be found in
>> >> "QorIQ P2020 Integrated Processor Reference Manual"
>> >> "Chapter 6 L2 Look-Aside Cache/SRAM"
>> >> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
>> >
>> >That's not the answer to my question :)
>> >
>> >These are big-endian, right?  Please mark them as such and access them
>> >properly with the correct functions.
>>
>> Yes, they are big-edian.
>> Does it work to add comments(about order and access functions) for the structure ahead of it?
>> And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {...};
>
>No, not comments, these should be of the type __be32, right?
>

Yes, understand. It's clear straight forward.
I will update those in patch v2.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14  8:39                 ` Wenhu Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14  8:39 UTC (permalink / raw)
  To: Greg KH; +Cc: linuxppc-dev, linux-kernel

>On Tue, Jun 14, 2022 at 07:53:46AM +0000, Wenhu Wang wrote:
>> >> >> +
>> >> >> +struct mpc85xx_l2ctlr {
>> >> >> +     u32     ctl;            /* 0x000 - L2 control */
>> >> >
>> >> >What is the endian of these u32 values?  You map them directly to
>> >> >memory, so they must be specified some way, right?  Please make it
>> >> >obvious what they are.
>> >> >
>> >>
>> >> Surely, the values should be u32 here, modified in v2
>> >> The controller info could be found in
>> >> "QorIQ P2020 Integrated Processor Reference Manual"
>> >> "Chapter 6 L2 Look-Aside Cache/SRAM"
>> >> See: http://m4udit.dinauz.org/P2020RM_rev0.pdf
>> >
>> >That's not the answer to my question :)
>> >
>> >These are big-endian, right?  Please mark them as such and access them
>> >properly with the correct functions.
>>
>> Yes, they are big-edian.
>> Does it work to add comments(about order and access functions) for the structure ahead of it?
>> And appending like "_be", "_access_be" or "_big_endian"? (struct mpc85xx_l2ctlr_be {...};
>
>No, not comments, these should be of the type __be32, right?
>

Yes, understand. It's clear straight forward.
I will update those in patch v2.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
  2022-06-09 10:28   ` Wang Wenhu
  (?)
@ 2022-06-14 10:45   ` Michael Ellerman
  2022-06-14 14:49       ` Christoph Hellwig
  2022-06-15  4:37     ` 回复: " Wenhu Wang
  -1 siblings, 2 replies; 52+ messages in thread
From: Michael Ellerman @ 2022-06-14 10:45 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, christophe.leroy
  Cc: linuxppc-dev, linux-kernel, Wang Wenhu

Wang Wenhu <wenhu.wang@hotmail.com> writes:
> The function ioremap_coherent may be called by modules such as
> fsl_85xx_cache_sram. So export it for access in other modules.

ioremap_coherent() is powerpc specific, and only has one other caller,
I'd like to remove it.

Does ioremap_cache() work for you?

cheers

> diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
> index 4f12504fb405..08a00dacef0b 100644
> --- a/arch/powerpc/mm/ioremap.c
> +++ b/arch/powerpc/mm/ioremap.c
> @@ -40,6 +40,7 @@ void __iomem *ioremap_coherent(phys_addr_t addr, unsigned long size)
>  		return iowa_ioremap(addr, size, prot, caller);
>  	return __ioremap_caller(addr, size, prot, caller);
>  }
> +EXPORT_SYMBOL(ioremap_coherent);
>  
>  void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
>  {
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  7:53             ` Wenhu Wang
@ 2022-06-14 14:40               ` Wenhu Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14 14:40 UTC (permalink / raw)
  To: Christophe Leroy, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel

>>>
>>> I looked at that patch.
>>>
>>> I don't think you can just drop the #ifdef in function
>>> __access_remote_vm() in mm/memory.c
>>>
>>> You have to replace it with something like:
>>>
>>>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>          break;
>>>
>>
>>
>>Another thing in that patch:
>>
>>By making generic_access_phys() a static inline, it means that everytime
>>you refer to the address of that function in a vm_operations_struct
>>struct, the compiler has to provide an outlined instance of the
>>function. It means you'll likely have several instances of a
>>generic_access_phys().
>>
>>What you could do instead is to add the following at the start of
>>generic_access_phys() in mm/memory.c :
>>
>>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>                return 0;
>>
>
>It is really a better chmoce, thanks for the advice.
>Multiple instances exist as you mentioned, the block returns 0 with no-op
>instance which makes no difference with the function return value.
>
>I will update the patch after a re-confirming.
>

I tried as adviced but when not defined, error happens on archectures such
as arm64. Actually the function generic_access_phys calls a lot of functions
that become undefined if we compile it with CONFIG_HAVE_IOREMAP_PROT disabled.
The archectures that support CONFIG_HAVE_IOREMAP_PROT are mips, x86, sh, arc,
s390, loongarch and powerpc.

So we may just define the function with static inline and add IS_ENABLED
condition branch in function __access_remote_vm in mm/memory.c. The executing
path breaks if CONFIG_HAVE_IOREMAP_PROT is disabled, and never goes into the
static no-op function.

In short, the static inline no-op function would never be executed, the only
difference is that there would be a lot of function code in compiled target.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14 14:40               ` Wenhu Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-14 14:40 UTC (permalink / raw)
  To: Christophe Leroy, Greg KH; +Cc: linuxppc-dev, linux-kernel

>>>
>>> I looked at that patch.
>>>
>>> I don't think you can just drop the #ifdef in function
>>> __access_remote_vm() in mm/memory.c
>>>
>>> You have to replace it with something like:
>>>
>>>      if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>          break;
>>>
>>
>>
>>Another thing in that patch:
>>
>>By making generic_access_phys() a static inline, it means that everytime
>>you refer to the address of that function in a vm_operations_struct
>>struct, the compiler has to provide an outlined instance of the
>>function. It means you'll likely have several instances of a
>>generic_access_phys().
>>
>>What you could do instead is to add the following at the start of
>>generic_access_phys() in mm/memory.c :
>>
>>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>                return 0;
>>
>
>It is really a better chmoce, thanks for the advice.
>Multiple instances exist as you mentioned, the block returns 0 with no-op
>instance which makes no difference with the function return value.
>
>I will update the patch after a re-confirming.
>

I tried as adviced but when not defined, error happens on archectures such
as arm64. Actually the function generic_access_phys calls a lot of functions
that become undefined if we compile it with CONFIG_HAVE_IOREMAP_PROT disabled.
The archectures that support CONFIG_HAVE_IOREMAP_PROT are mips, x86, sh, arc,
s390, loongarch and powerpc.

So we may just define the function with static inline and add IS_ENABLED
condition branch in function __access_remote_vm in mm/memory.c. The executing
path breaks if CONFIG_HAVE_IOREMAP_PROT is disabled, and never goes into the
static no-op function.

In short, the static inline no-op function would never be executed, the only
difference is that there would be a lot of function code in compiled target.

Thanks,
Wenhu

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
  2022-06-14 10:45   ` Michael Ellerman
@ 2022-06-14 14:49       ` Christoph Hellwig
  2022-06-15  4:37     ` 回复: " Wenhu Wang
  1 sibling, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-14 14:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Wang Wenhu, gregkh, christophe.leroy, linuxppc-dev, linux-kernel

On Tue, Jun 14, 2022 at 08:45:25PM +1000, Michael Ellerman wrote:
> Wang Wenhu <wenhu.wang@hotmail.com> writes:
> > The function ioremap_coherent may be called by modules such as
> > fsl_85xx_cache_sram. So export it for access in other modules.
> 
> ioremap_coherent() is powerpc specific, and only has one other caller,
> I'd like to remove it.
> 
> Does ioremap_cache() work for you?

Chances are that both are the wrong thing and this really wants
memremap, as SRAM tends to have memory and not MMIO semantics.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
@ 2022-06-14 14:49       ` Christoph Hellwig
  0 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-14 14:49 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: gregkh, linuxppc-dev, Wang Wenhu, linux-kernel

On Tue, Jun 14, 2022 at 08:45:25PM +1000, Michael Ellerman wrote:
> Wang Wenhu <wenhu.wang@hotmail.com> writes:
> > The function ioremap_coherent may be called by modules such as
> > fsl_85xx_cache_sram. So export it for access in other modules.
> 
> ioremap_coherent() is powerpc specific, and only has one other caller,
> I'd like to remove it.
> 
> Does ioremap_cache() work for you?

Chances are that both are the wrong thing and this really wants
memremap, as SRAM tends to have memory and not MMIO semantics.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-09 10:28   ` Wang Wenhu
@ 2022-06-14 14:51     ` Christoph Hellwig
  -1 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-14 14:51 UTC (permalink / raw)
  To: Wang Wenhu; +Cc: gregkh, christophe.leroy, linuxppc-dev, linux-kernel

UIO seems like the wrong kind of interface for this.  Why isn't this
a simple character device?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14 14:51     ` Christoph Hellwig
  0 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-14 14:51 UTC (permalink / raw)
  To: Wang Wenhu; +Cc: gregkh, linuxppc-dev, linux-kernel

UIO seems like the wrong kind of interface for this.  Why isn't this
a simple character device?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14 14:40               ` Wenhu Wang
@ 2022-06-14 14:59                 ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14 14:59 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel



Le 14/06/2022 à 16:40, Wenhu Wang a écrit :
>>>>
>>>> I looked at that patch.
>>>>
>>>> I don't think you can just drop the #ifdef in function
>>>> __access_remote_vm() in mm/memory.c
>>>>
>>>> You have to replace it with something like:
>>>>
>>>>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>>            break;
>>>>
>>>
>>>
>>> Another thing in that patch:
>>>
>>> By making generic_access_phys() a static inline, it means that everytime
>>> you refer to the address of that function in a vm_operations_struct
>>> struct, the compiler has to provide an outlined instance of the
>>> function. It means you'll likely have several instances of a
>>> generic_access_phys().
>>>
>>> What you could do instead is to add the following at the start of
>>> generic_access_phys() in mm/memory.c :
>>>
>>>          if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>                  return 0;
>>>
>>
>> It is really a better chmoce, thanks for the advice.
>> Multiple instances exist as you mentioned, the block returns 0 with no-op
>> instance which makes no difference with the function return value.
>>
>> I will update the patch after a re-confirming.
>>
> 
> I tried as adviced but when not defined, error happens on archectures such
> as arm64. Actually the function generic_access_phys calls a lot of functions
> that become undefined if we compile it with CONFIG_HAVE_IOREMAP_PROT disabled.
> The archectures that support CONFIG_HAVE_IOREMAP_PROT are mips, x86, sh, arc,
> s390, loongarch and powerpc.
> 
> So we may just define the function with static inline and add IS_ENABLED
> condition branch in function __access_remote_vm in mm/memory.c. The executing
> path breaks if CONFIG_HAVE_IOREMAP_PROT is disabled, and never goes into the
> static no-op function.
> 
> In short, the static inline no-op function would never be executed, the only
> difference is that there would be a lot of function code in compiled target.
> 

In that case all you have to do is:

diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..39b369fc77f6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5413,6 +5413,13 @@ int generic_access_phys(struct vm_area_struct 
*vma, unsigned long addr,
  	return ret;
  }
  EXPORT_SYMBOL_GPL(generic_access_phys);
+#else
+int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(generic_access_phys);
  #endif

  /*


Christophe

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-14 14:59                 ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-14 14:59 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: linuxppc-dev, linux-kernel



Le 14/06/2022 à 16:40, Wenhu Wang a écrit :
>>>>
>>>> I looked at that patch.
>>>>
>>>> I don't think you can just drop the #ifdef in function
>>>> __access_remote_vm() in mm/memory.c
>>>>
>>>> You have to replace it with something like:
>>>>
>>>>        if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>>            break;
>>>>
>>>
>>>
>>> Another thing in that patch:
>>>
>>> By making generic_access_phys() a static inline, it means that everytime
>>> you refer to the address of that function in a vm_operations_struct
>>> struct, the compiler has to provide an outlined instance of the
>>> function. It means you'll likely have several instances of a
>>> generic_access_phys().
>>>
>>> What you could do instead is to add the following at the start of
>>> generic_access_phys() in mm/memory.c :
>>>
>>>          if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
>>>                  return 0;
>>>
>>
>> It is really a better chmoce, thanks for the advice.
>> Multiple instances exist as you mentioned, the block returns 0 with no-op
>> instance which makes no difference with the function return value.
>>
>> I will update the patch after a re-confirming.
>>
> 
> I tried as adviced but when not defined, error happens on archectures such
> as arm64. Actually the function generic_access_phys calls a lot of functions
> that become undefined if we compile it with CONFIG_HAVE_IOREMAP_PROT disabled.
> The archectures that support CONFIG_HAVE_IOREMAP_PROT are mips, x86, sh, arc,
> s390, loongarch and powerpc.
> 
> So we may just define the function with static inline and add IS_ENABLED
> condition branch in function __access_remote_vm in mm/memory.c. The executing
> path breaks if CONFIG_HAVE_IOREMAP_PROT is disabled, and never goes into the
> static no-op function.
> 
> In short, the static inline no-op function would never be executed, the only
> difference is that there would be a lot of function code in compiled target.
> 

In that case all you have to do is:

diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..39b369fc77f6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5413,6 +5413,13 @@ int generic_access_phys(struct vm_area_struct 
*vma, unsigned long addr,
  	return ret;
  }
  EXPORT_SYMBOL_GPL(generic_access_phys);
+#else
+int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(generic_access_phys);
  #endif

  /*


Christophe

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* 回复: [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent
  2022-06-14 10:45   ` Michael Ellerman
  2022-06-14 14:49       ` Christoph Hellwig
@ 2022-06-15  4:37     ` Wenhu Wang
  1 sibling, 0 replies; 52+ messages in thread
From: Wenhu Wang @ 2022-06-15  4:37 UTC (permalink / raw)
  To: Michael Ellerman, gregkh, christophe.leroy; +Cc: linuxppc-dev, linux-kernel

>发件人: Michael Ellerman <mpe@ellerman.id.au>
>发送时间: 2022年6月14日 18:45
>收件人: Wang Wenhu <wenhu.wang@hotmail.com>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; christophe.leroy@csgroup.eu <christophe.leroy@csgroup.eu>
>抄送: linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Wang Wenhu <wenhu.wang@hotmail.com>
>主题: Re: [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent 
> 
>Wang Wenhu <wenhu.wang@hotmail.com> writes:
>> The function ioremap_coherent may be called by modules such as
>> fsl_85xx_cache_sram. So export it for access in other modules.
>
>ioremap_coherent() is powerpc specific, and only has one other caller,
>I'd like to remove it.
>
>Does ioremap_cache() work for you?
>

Yes, it works. I will update in v2 to use ioremap_cache.
I tested and compared the outcomes of ioremap_cache and ioremap_coherent,
and found they ended same values.

Thanks,
Wenhu

>
>> diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
>> index 4f12504fb405..08a00dacef0b 100644
>> --- a/arch/powerpc/mm/ioremap.c
>> +++ b/arch/powerpc/mm/ioremap.c
>> @@ -40,6 +40,7 @@ void __iomem *ioremap_coherent(phys_addr_t addr, unsigned long size)
>>                return iowa_ioremap(addr, size, prot, caller);
>>        return __ioremap_caller(addr, size, prot, caller);
>>  }
>> +EXPORT_SYMBOL(ioremap_coherent);
>>  
>>  void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
>  {
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCHv2 0/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver
  2022-06-09 13:17     ` Greg KH
@ 2022-06-15  5:57       ` Wang Wenhu
  -1 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: christophe.leroy, linux-kernel, linuxppc-dev, mpe, wenhu.wang

This series try to push an uio driver which works on freescale mpc85xx
to configure its l2-cache-sram as a block of SRAM and enable user level
application access of the SRAM.

1/2: For coding-style consideration of macro CONFIG_HAVE_IOREMAP_PORT;
2/2: Implementation of the uio driver.

This is the second version, which addressed some commets:
1. Use __be32 instead of u32 for the big-endian data declarations;
2. Remove "static inline" version of generic_access_phys definition in .h file
and give a version of no-op definition in mm/memory.c;
3. Use generic ioremap_cache instead of ioremap_coherent

For v1, see:
1/2: https://lore.kernel.org/all/20220610144348.GA595923@bhelgaas/T/
2/2: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/

Wang Wenhu (2):
  mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation

 drivers/char/mem.c                    |   2 -
 drivers/fpga/dfl-afu-main.c           |   2 -
 drivers/pci/mmap.c                    |   2 -
 drivers/uio/Kconfig                   |  14 ++
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio.c                     |   2 -
 drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
 mm/memory.c                           |  13 +-
 8 files changed, 312 insertions(+), 12 deletions(-)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCHv2 0/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver
@ 2022-06-15  5:57       ` Wang Wenhu
  0 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: linuxppc-dev, linux-kernel, wenhu.wang

This series try to push an uio driver which works on freescale mpc85xx
to configure its l2-cache-sram as a block of SRAM and enable user level
application access of the SRAM.

1/2: For coding-style consideration of macro CONFIG_HAVE_IOREMAP_PORT;
2/2: Implementation of the uio driver.

This is the second version, which addressed some commets:
1. Use __be32 instead of u32 for the big-endian data declarations;
2. Remove "static inline" version of generic_access_phys definition in .h file
and give a version of no-op definition in mm/memory.c;
3. Use generic ioremap_cache instead of ioremap_coherent

For v1, see:
1/2: https://lore.kernel.org/all/20220610144348.GA595923@bhelgaas/T/
2/2: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/

Wang Wenhu (2):
  mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation

 drivers/char/mem.c                    |   2 -
 drivers/fpga/dfl-afu-main.c           |   2 -
 drivers/pci/mmap.c                    |   2 -
 drivers/uio/Kconfig                   |  14 ++
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio.c                     |   2 -
 drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
 mm/memory.c                           |  13 +-
 8 files changed, 312 insertions(+), 12 deletions(-)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
       [not found]     ` <20220615055735.53585-1-wenhu.wang@hotmail.com>
@ 2022-06-15  5:57         ` Wang Wenhu
  2022-06-15  5:57         ` Wang Wenhu
  1 sibling, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: christophe.leroy, linux-kernel, linuxppc-dev, mpe, wenhu.wang

It is recommended in the "Conditional Compilation" chapter of kernel
coding-style documentation that preprocessor conditionals should not
be used in .c files wherever possible.

As for the macro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
to eliminate it in .c files which are referencers. We constrict its usage
only to mm/memory.c.
HAVE_IOREMAP_PROT is supported by part of archectures such as powerpc and
x86, but not supported by some others such as arm. So for some functions,
a no-op version should be available. Currently it's generic_access_phys,
which is referenced by some other modules.

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
v2:
 - Added IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT) condition in __access_remote_vm
 - Added generic_access_phys() function with no-op in mm/memory.c instead of the 
 former one of "static inline" in include/linux/mm.h
Former: https://lore.kernel.org/linux-mm/YqMRtWAH5fIWsLQB@kroah.com/T/
---
 drivers/char/mem.c          |  2 --
 drivers/fpga/dfl-afu-main.c |  2 --
 drivers/pci/mmap.c          |  2 --
 drivers/uio/uio.c           |  2 --
 mm/memory.c                 | 13 +++++++++----
 5 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 84ca98ed1dad..40186a441e38 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
 #endif
 
 static const struct vm_operations_struct mmap_mem_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys
-#endif
 };
 
 static int mmap_mem(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 7f621e96d3b8..833e14806c7a 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 }
 
 static const struct vm_operations_struct afu_vma_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
index b8c9011987f4..1dcfabf80453 100644
--- a/drivers/pci/mmap.c
+++ b/drivers/pci/mmap.c
@@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
 #endif
 
 static const struct vm_operations_struct pci_phys_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 43afbb7c5ab9..c9205a121007 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
 }
 
 static const struct vm_operations_struct uio_physical_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int uio_mmap_physical(struct vm_area_struct *vma)
diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..7c0e59085456 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5413,6 +5413,13 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(generic_access_phys);
+#else
+int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(generic_access_phys);
 #endif
 
 /*
@@ -5437,9 +5444,8 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 		ret = get_user_pages_remote(mm, addr, 1,
 				gup_flags, &page, &vma, NULL);
 		if (ret <= 0) {
-#ifndef CONFIG_HAVE_IOREMAP_PROT
-			break;
-#else
+			if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
+				break;
 			/*
 			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
 			 * we can access using slightly different code.
@@ -5453,7 +5459,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 			if (ret <= 0)
 				break;
 			bytes = ret;
-#endif
 		} else {
 			bytes = len;
 			offset = addr & (PAGE_SIZE-1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
@ 2022-06-15  5:57         ` Wang Wenhu
  0 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: linuxppc-dev, linux-kernel, wenhu.wang

It is recommended in the "Conditional Compilation" chapter of kernel
coding-style documentation that preprocessor conditionals should not
be used in .c files wherever possible.

As for the macro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
to eliminate it in .c files which are referencers. We constrict its usage
only to mm/memory.c.
HAVE_IOREMAP_PROT is supported by part of archectures such as powerpc and
x86, but not supported by some others such as arm. So for some functions,
a no-op version should be available. Currently it's generic_access_phys,
which is referenced by some other modules.

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
v2:
 - Added IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT) condition in __access_remote_vm
 - Added generic_access_phys() function with no-op in mm/memory.c instead of the 
 former one of "static inline" in include/linux/mm.h
Former: https://lore.kernel.org/linux-mm/YqMRtWAH5fIWsLQB@kroah.com/T/
---
 drivers/char/mem.c          |  2 --
 drivers/fpga/dfl-afu-main.c |  2 --
 drivers/pci/mmap.c          |  2 --
 drivers/uio/uio.c           |  2 --
 mm/memory.c                 | 13 +++++++++----
 5 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 84ca98ed1dad..40186a441e38 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
 #endif
 
 static const struct vm_operations_struct mmap_mem_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys
-#endif
 };
 
 static int mmap_mem(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 7f621e96d3b8..833e14806c7a 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 }
 
 static const struct vm_operations_struct afu_vma_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
index b8c9011987f4..1dcfabf80453 100644
--- a/drivers/pci/mmap.c
+++ b/drivers/pci/mmap.c
@@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
 #endif
 
 static const struct vm_operations_struct pci_phys_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 43afbb7c5ab9..c9205a121007 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
 }
 
 static const struct vm_operations_struct uio_physical_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int uio_mmap_physical(struct vm_area_struct *vma)
diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..7c0e59085456 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5413,6 +5413,13 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(generic_access_phys);
+#else
+int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(generic_access_phys);
 #endif
 
 /*
@@ -5437,9 +5444,8 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 		ret = get_user_pages_remote(mm, addr, 1,
 				gup_flags, &page, &vma, NULL);
 		if (ret <= 0) {
-#ifndef CONFIG_HAVE_IOREMAP_PROT
-			break;
-#else
+			if (!IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT))
+				break;
 			/*
 			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
 			 * we can access using slightly different code.
@@ -5453,7 +5459,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 			if (ret <= 0)
 				break;
 			bytes = ret;
-#endif
 		} else {
 			bytes = len;
 			offset = addr & (PAGE_SIZE-1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
       [not found]     ` <20220615055735.53585-1-wenhu.wang@hotmail.com>
@ 2022-06-15  5:57         ` Wang Wenhu
  2022-06-15  5:57         ` Wang Wenhu
  1 sibling, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: christophe.leroy, linux-kernel, linuxppc-dev, mpe, wenhu.wang

Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
or fully. Users can make use of it as a block of independent memory that
offers special usage, such as for debuging or other critical status info
storage, which keeps consistently even when the whole system crashed.
Applications can make use of UIO driver to access the SRAM from user level.

Once there was another driver version for the l2-cache-sram for SRAM access
in kernel space. It had been removed recently.
See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
v2:
 - Use __be32 instead of u32 for big-endian data declarations;
 - Use generic ioremap_cache instead of ioremap_coherent;
 - Physical address support both 32 and 64 bits;
 - Addressed some other comments from Greg.
---
 drivers/uio/Kconfig                   |  14 ++
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
 3 files changed, 303 insertions(+)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 2e16c5338e5b..f7604584a12c 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,20 @@ config UIO_NETX
 	  To compile this driver as a module, choose M here; the module
 	  will be called uio_netx.
 
+config UIO_FSL_85XX_CACHE_SRAM
+	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
+	depends on FSL_SOC_BOOKE && PPC32
+	help
+	  Driver for user level access of freescale mpc85xx l2-cache-sram.
+
+	  Freescale's mpc85xx provides an option of configuring a part of
+	  (or full) cache memory as SRAM. The driver does this configuring
+	  work and exports SRAM to user-space for access form user level.
+	  This is extremely helpful for user applications that require
+	  high performance memory accesses.
+
+	  If you don't know what to do here, say N.
+
 config UIO_FSL_ELBC_GPCM
 	tristate "eLBC/GPCM driver"
 	depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index f2f416a14228..1ba07d92a1b1 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
 obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index 000000000000..6f91b0aa946b
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
+ * All rights reserved.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/stringify.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
+#define UIO_INFO_VER	"0.0.1"
+#define UIO_NAME	"uio_cache_sram"
+
+#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
+#define L2CR_L2IO		0x00200000	/* L2 instruction only */
+#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
+#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
+#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
+#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
+#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
+#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
+#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
+#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
+
+#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
+
+#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
+#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
+
+enum cache_sram_lock_ways {
+	LOCK_WAYS_ZERO		= 0,
+	LOCK_WAYS_EIGHTH	= 1,
+	LOCK_WAYS_TWO_EIGHTH	= 2,
+	LOCK_WAYS_HALF		= 4,
+	LOCK_WAYS_FULL		= 8,
+};
+
+struct mpc85xx_l2ctlr {
+	__be32	ctl;		/* 0x000 - L2 control */
+	u8	res1[0xC];
+	__be32	ewar0;		/* 0x010 - External write address 0 */
+	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
+	__be32	ewcr0;		/* 0x018 - External write ctrl */
+	u8	res2[4];
+	__be32	ewar1;		/* 0x020 - External write address 1 */
+	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
+	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
+	u8	res3[4];
+	__be32	ewar2;		/* 0x030 - External write address 2 */
+	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
+	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
+	u8	res4[4];
+	__be32	ewar3;		/* 0x040 - External write address 3 */
+	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
+	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
+	u8	res5[0xB4];
+	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
+	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
+	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
+	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
+	u8	res6[0xCF0];
+	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
+	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
+	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
+	u8	res7[0x14];
+	__be32	captdatahi;	/* 0xE20 - Error data high capture */
+	__be32	captdatalo;	/* 0xE24 - Error data low capture */
+	__be32	captecc;	/* 0xE28 - Error syndrome */
+	u8	res8[0x14];
+	__be32	errdet;		/* 0xE40 - Error detect */
+	__be32	errdis;		/* 0xE44 - Error disable */
+	__be32	errinten;	/* 0xE48 - Error interrupt enable */
+	__be32	errattr;	/* 0xE4c - Error attribute capture */
+	__be32	erradrrl;	/* 0xE50 - Error address capture low */
+	__be32	erradrrh;	/* 0xE54 - Error address capture high */
+	__be32	errctl;		/* 0xE58 - Error control */
+	u8	res9[0x1A4];
+};
+
+static int uio_cache_sram_setup(struct platform_device *pdev,
+				phys_addr_t base, u8 ways)
+{
+	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
+
+	if (!l2ctlr) {
+		dev_err(&pdev->dev, "can not map l2 controller\n");
+		return -EINVAL;
+	}
+
+	/* write bits[0-17] to srbar0 */
+	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
+
+	/* write bits[18-21] to srbare0 */
+	if (IS_ENABLED(CONFIG_PHYS_64BIT))
+		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
+
+	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
+
+	switch (ways) {
+	case LOCK_WAYS_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
+		break;
+
+	case LOCK_WAYS_TWO_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
+		break;
+
+	case LOCK_WAYS_HALF:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
+		break;
+
+	case LOCK_WAYS_FULL:
+	default:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
+		break;
+	}
+	eieio();
+
+	return 0;
+}
+
+static const struct vm_operations_struct uio_cache_sram_vm_ops = {
+	.access = generic_access_phys,
+};
+
+static int uio_cache_sram_mmap(struct uio_info *info,
+			       struct vm_area_struct *vma)
+{
+	struct uio_mem *mem = info->mem;
+
+	if (mem->addr & ~PAGE_MASK)
+		return -ENODEV;
+
+	if ((vma->vm_end - vma->vm_start > mem->size) ||
+	    (mem->size == 0) ||
+	    (mem->memtype != UIO_MEM_PHYS))
+		return -EINVAL;
+
+	vma->vm_ops = &uio_cache_sram_vm_ops;
+	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
+
+	return remap_pfn_range(vma,
+			       vma->vm_start,
+			       mem->addr >> PAGE_SHIFT,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot);
+}
+
+static int uio_cache_sram_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct uio_info *info;
+	struct uio_mem *uiomem;
+	const char *dt_name;
+	u32 mem_base_32;
+	u64 mem_base_64;
+	phys_addr_t mem_base;
+	u32 l2cache_size;
+	u32 mem_size;
+	u32 rem;
+	u8 ways;
+	int ret;
+
+	/* alloc uio_info for one device */
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	/* get optional uio name */
+	if (of_property_read_string(node, "uio_name", &dt_name))
+		dt_name = UIO_NAME;
+
+	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
+	if (!info->name)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-size\n");
+		return -EINVAL;
+	}
+
+
+	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
+		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
+		mem_base = mem_base_64;
+	} else {
+		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
+		mem_base = mem_base_32;
+	}
+
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-base\n");
+		return -EINVAL;
+	}
+
+	if (mem_size == 0) {
+		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing l2cache-size\n");
+		return -EINVAL;
+	}
+
+	rem = l2cache_size % mem_size;
+	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
+	if (rem || (ways & (ways - 1))) {
+		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
+		return -EINVAL;
+	}
+
+	ret = uio_cache_sram_setup(pdev, mem_base, ways);
+	if (ret)
+		return ret;
+
+	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
+		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
+		ret = -ENXIO;
+	}
+
+	info->irq = UIO_IRQ_NONE;
+	info->version = UIO_INFO_VER;
+	info->mmap = uio_cache_sram_mmap;
+	uiomem = info->mem;
+	uiomem->memtype = UIO_MEM_PHYS;
+	uiomem->addr = mem_base;
+	uiomem->size = mem_size;
+	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
+	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
+	if (!uiomem->internal_addr) {
+		dev_err(&pdev->dev, "cache ioremap failed\n");
+		ret = -ENOMEM;
+	}
+
+	/* register uio device */
+	if (uio_register_device(&pdev->dev, info)) {
+		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
+		ret = -ENODEV;
+		goto err_out;
+	}
+
+	platform_set_drvdata(pdev, info);
+	return 0;
+
+err_out:
+	iounmap(info->mem[0].internal_addr);
+	return ret;
+}
+
+static int uio_cache_sram_remove(struct platform_device *pdev)
+{
+	struct uio_info *info = platform_get_drvdata(pdev);
+
+	uio_unregister_device(info);
+	iounmap(info->mem[0].internal_addr);
+
+	return 0;
+}
+
+static const struct of_device_id uio_cache_sram_of_match[] = {
+	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
+
+static struct platform_driver uio_fsl_85xx_cache_sram = {
+	.probe = uio_cache_sram_probe,
+	.remove = uio_cache_sram_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table	= uio_cache_sram_of_match,
+	},
+};
+
+module_platform_driver(uio_fsl_85xx_cache_sram);
+
+MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
+MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-15  5:57         ` Wang Wenhu
  0 siblings, 0 replies; 52+ messages in thread
From: Wang Wenhu @ 2022-06-15  5:57 UTC (permalink / raw)
  To: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm
  Cc: linuxppc-dev, linux-kernel, wenhu.wang

Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
or fully. Users can make use of it as a block of independent memory that
offers special usage, such as for debuging or other critical status info
storage, which keeps consistently even when the whole system crashed.
Applications can make use of UIO driver to access the SRAM from user level.

Once there was another driver version for the l2-cache-sram for SRAM access
in kernel space. It had been removed recently.
See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03

Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
---
v2:
 - Use __be32 instead of u32 for big-endian data declarations;
 - Use generic ioremap_cache instead of ioremap_coherent;
 - Physical address support both 32 and 64 bits;
 - Addressed some other comments from Greg.
---
 drivers/uio/Kconfig                   |  14 ++
 drivers/uio/Makefile                  |   1 +
 drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
 3 files changed, 303 insertions(+)
 create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 2e16c5338e5b..f7604584a12c 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -105,6 +105,20 @@ config UIO_NETX
 	  To compile this driver as a module, choose M here; the module
 	  will be called uio_netx.
 
+config UIO_FSL_85XX_CACHE_SRAM
+	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
+	depends on FSL_SOC_BOOKE && PPC32
+	help
+	  Driver for user level access of freescale mpc85xx l2-cache-sram.
+
+	  Freescale's mpc85xx provides an option of configuring a part of
+	  (or full) cache memory as SRAM. The driver does this configuring
+	  work and exports SRAM to user-space for access form user level.
+	  This is extremely helpful for user applications that require
+	  high performance memory accesses.
+
+	  If you don't know what to do here, say N.
+
 config UIO_FSL_ELBC_GPCM
 	tristate "eLBC/GPCM driver"
 	depends on FSL_LBC
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index f2f416a14228..1ba07d92a1b1 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
 obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
 obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
+obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
new file mode 100644
index 000000000000..6f91b0aa946b
--- /dev/null
+++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
+ * All rights reserved.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/stringify.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
+#define UIO_INFO_VER	"0.0.1"
+#define UIO_NAME	"uio_cache_sram"
+
+#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
+#define L2CR_L2IO		0x00200000	/* L2 instruction only */
+#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
+#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
+#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
+#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
+#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
+#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
+#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
+#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
+
+#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
+
+#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
+#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
+
+enum cache_sram_lock_ways {
+	LOCK_WAYS_ZERO		= 0,
+	LOCK_WAYS_EIGHTH	= 1,
+	LOCK_WAYS_TWO_EIGHTH	= 2,
+	LOCK_WAYS_HALF		= 4,
+	LOCK_WAYS_FULL		= 8,
+};
+
+struct mpc85xx_l2ctlr {
+	__be32	ctl;		/* 0x000 - L2 control */
+	u8	res1[0xC];
+	__be32	ewar0;		/* 0x010 - External write address 0 */
+	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
+	__be32	ewcr0;		/* 0x018 - External write ctrl */
+	u8	res2[4];
+	__be32	ewar1;		/* 0x020 - External write address 1 */
+	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
+	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
+	u8	res3[4];
+	__be32	ewar2;		/* 0x030 - External write address 2 */
+	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
+	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
+	u8	res4[4];
+	__be32	ewar3;		/* 0x040 - External write address 3 */
+	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
+	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
+	u8	res5[0xB4];
+	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
+	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
+	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
+	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
+	u8	res6[0xCF0];
+	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
+	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
+	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
+	u8	res7[0x14];
+	__be32	captdatahi;	/* 0xE20 - Error data high capture */
+	__be32	captdatalo;	/* 0xE24 - Error data low capture */
+	__be32	captecc;	/* 0xE28 - Error syndrome */
+	u8	res8[0x14];
+	__be32	errdet;		/* 0xE40 - Error detect */
+	__be32	errdis;		/* 0xE44 - Error disable */
+	__be32	errinten;	/* 0xE48 - Error interrupt enable */
+	__be32	errattr;	/* 0xE4c - Error attribute capture */
+	__be32	erradrrl;	/* 0xE50 - Error address capture low */
+	__be32	erradrrh;	/* 0xE54 - Error address capture high */
+	__be32	errctl;		/* 0xE58 - Error control */
+	u8	res9[0x1A4];
+};
+
+static int uio_cache_sram_setup(struct platform_device *pdev,
+				phys_addr_t base, u8 ways)
+{
+	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
+
+	if (!l2ctlr) {
+		dev_err(&pdev->dev, "can not map l2 controller\n");
+		return -EINVAL;
+	}
+
+	/* write bits[0-17] to srbar0 */
+	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
+
+	/* write bits[18-21] to srbare0 */
+	if (IS_ENABLED(CONFIG_PHYS_64BIT))
+		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
+
+	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
+
+	switch (ways) {
+	case LOCK_WAYS_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
+		break;
+
+	case LOCK_WAYS_TWO_EIGHTH:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
+		break;
+
+	case LOCK_WAYS_HALF:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
+		break;
+
+	case LOCK_WAYS_FULL:
+	default:
+		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
+		break;
+	}
+	eieio();
+
+	return 0;
+}
+
+static const struct vm_operations_struct uio_cache_sram_vm_ops = {
+	.access = generic_access_phys,
+};
+
+static int uio_cache_sram_mmap(struct uio_info *info,
+			       struct vm_area_struct *vma)
+{
+	struct uio_mem *mem = info->mem;
+
+	if (mem->addr & ~PAGE_MASK)
+		return -ENODEV;
+
+	if ((vma->vm_end - vma->vm_start > mem->size) ||
+	    (mem->size == 0) ||
+	    (mem->memtype != UIO_MEM_PHYS))
+		return -EINVAL;
+
+	vma->vm_ops = &uio_cache_sram_vm_ops;
+	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
+
+	return remap_pfn_range(vma,
+			       vma->vm_start,
+			       mem->addr >> PAGE_SHIFT,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot);
+}
+
+static int uio_cache_sram_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct uio_info *info;
+	struct uio_mem *uiomem;
+	const char *dt_name;
+	u32 mem_base_32;
+	u64 mem_base_64;
+	phys_addr_t mem_base;
+	u32 l2cache_size;
+	u32 mem_size;
+	u32 rem;
+	u8 ways;
+	int ret;
+
+	/* alloc uio_info for one device */
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	/* get optional uio name */
+	if (of_property_read_string(node, "uio_name", &dt_name))
+		dt_name = UIO_NAME;
+
+	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
+	if (!info->name)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-size\n");
+		return -EINVAL;
+	}
+
+
+	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
+		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
+		mem_base = mem_base_64;
+	} else {
+		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
+		mem_base = mem_base_32;
+	}
+
+	if (ret) {
+		dev_err(&pdev->dev, "missing cache-mem-base\n");
+		return -EINVAL;
+	}
+
+	if (mem_size == 0) {
+		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
+	if (ret) {
+		dev_err(&pdev->dev, "missing l2cache-size\n");
+		return -EINVAL;
+	}
+
+	rem = l2cache_size % mem_size;
+	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
+	if (rem || (ways & (ways - 1))) {
+		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
+		return -EINVAL;
+	}
+
+	ret = uio_cache_sram_setup(pdev, mem_base, ways);
+	if (ret)
+		return ret;
+
+	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
+		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
+		ret = -ENXIO;
+	}
+
+	info->irq = UIO_IRQ_NONE;
+	info->version = UIO_INFO_VER;
+	info->mmap = uio_cache_sram_mmap;
+	uiomem = info->mem;
+	uiomem->memtype = UIO_MEM_PHYS;
+	uiomem->addr = mem_base;
+	uiomem->size = mem_size;
+	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
+	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
+	if (!uiomem->internal_addr) {
+		dev_err(&pdev->dev, "cache ioremap failed\n");
+		ret = -ENOMEM;
+	}
+
+	/* register uio device */
+	if (uio_register_device(&pdev->dev, info)) {
+		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
+		ret = -ENODEV;
+		goto err_out;
+	}
+
+	platform_set_drvdata(pdev, info);
+	return 0;
+
+err_out:
+	iounmap(info->mem[0].internal_addr);
+	return ret;
+}
+
+static int uio_cache_sram_remove(struct platform_device *pdev)
+{
+	struct uio_info *info = platform_get_drvdata(pdev);
+
+	uio_unregister_device(info);
+	iounmap(info->mem[0].internal_addr);
+
+	return 0;
+}
+
+static const struct of_device_id uio_cache_sram_of_match[] = {
+	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
+
+static struct platform_driver uio_fsl_85xx_cache_sram = {
+	.probe = uio_cache_sram_probe,
+	.remove = uio_cache_sram_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table	= uio_cache_sram_of_match,
+	},
+};
+
+module_platform_driver(uio_fsl_85xx_cache_sram);
+
+MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
+MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-14  6:09       ` Wenhu Wang
@ 2022-06-15  6:04         ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:04 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: mpe, linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>> +
>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>
>> Same here.
>>
> 
> I tried to eliminate it in mainline
> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
> https://lkml.org/lkml/2022/6/10/695
> 
>>> +     .access = generic_access_phys,
>>> +#endif
>>> +};

Another solution is to do:


static const struct vm_operations_struct uio_cache_sram_vm_ops = {
	.access = IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT) ? generic_access_phys : 
NULL,
};


Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-15  6:04         ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:04 UTC (permalink / raw)
  To: Wenhu Wang, Greg KH; +Cc: linuxppc-dev, linux-kernel



Le 14/06/2022 à 08:09, Wenhu Wang a écrit :
>>> +
>>> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
>>> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>>
>> Same here.
>>
> 
> I tried to eliminate it in mainline
> See: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
> https://lkml.org/lkml/2022/6/10/695
> 
>>> +     .access = generic_access_phys,
>>> +#endif
>>> +};

Another solution is to do:


static const struct vm_operations_struct uio_cache_sram_vm_ops = {
	.access = IS_ENABLED(CONFIG_HAVE_IOREMAP_PROT) ? generic_access_phys : 
NULL,
};


Christophe

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-15  6:43           ` Christoph Hellwig
  -1 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-15  6:43 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm, christophe.leroy, linux-kernel,
	linuxppc-dev, mpe

As pointed out last time:  uio is the wrong interface to expose sram,
and any kind of ioremap is the wrong way to map it.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-15  6:43           ` Christoph Hellwig
  0 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-15  6:43 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: arnd, gregkh, linux-fpga, linux-pci, linux-kernel, linux-mm, mdf,
	trix, bhelgaas, akpm, linuxppc-dev, yilun.xu, hao.wu

As pointed out last time:  uio is the wrong interface to expose sram,
and any kind of ioremap is the wrong way to map it.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-15  6:48           ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:48 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: linux-kernel, linuxppc-dev, mpe



Le 15/06/2022 à 07:57, Wang Wenhu a écrit :
> Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
> or fully. Users can make use of it as a block of independent memory that
> offers special usage, such as for debuging or other critical status info
> storage, which keeps consistently even when the whole system crashed.
> Applications can make use of UIO driver to access the SRAM from user level.
> 
> Once there was another driver version for the l2-cache-sram for SRAM access
> in kernel space. It had been removed recently.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
> v2:
>   - Use __be32 instead of u32 for big-endian data declarations;
>   - Use generic ioremap_cache instead of ioremap_coherent;
>   - Physical address support both 32 and 64 bits;
>   - Addressed some other comments from Greg.
> ---
>   drivers/uio/Kconfig                   |  14 ++
>   drivers/uio/Makefile                  |   1 +
>   drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
>   3 files changed, 303 insertions(+)
>   create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..f7604584a12c 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,20 @@ config UIO_NETX
>   	  To compile this driver as a module, choose M here; the module
>   	  will be called uio_netx.
>   
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Driver for user level access of freescale mpc85xx l2-cache-sram.
> +
> +	  Freescale's mpc85xx provides an option of configuring a part of
> +	  (or full) cache memory as SRAM. The driver does this configuring
> +	  work and exports SRAM to user-space for access form user level.
> +	  This is extremely helpful for user applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.
> +
>   config UIO_FSL_ELBC_GPCM
>   	tristate "eLBC/GPCM driver"
>   	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>   obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>   obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>   obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..6f91b0aa946b
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME	"uio_cache_sram"
> +
> +#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO		0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO		= 0,
> +	LOCK_WAYS_EIGHTH	= 1,
> +	LOCK_WAYS_TWO_EIGHTH	= 2,
> +	LOCK_WAYS_HALF		= 4,
> +	LOCK_WAYS_FULL		= 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	__be32	ctl;		/* 0x000 - L2 control */
> +	u8	res1[0xC];
> +	__be32	ewar0;		/* 0x010 - External write address 0 */
> +	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	__be32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	__be32	ewar1;		/* 0x020 - External write address 1 */
> +	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	__be32	ewar2;		/* 0x030 - External write address 2 */
> +	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	__be32	ewar3;		/* 0x040 - External write address 3 */
> +	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	__be32	captdatahi;	/* 0xE20 - Error data high capture */
> +	__be32	captdatalo;	/* 0xE24 - Error data low capture */
> +	__be32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	__be32	errdet;		/* 0xE40 - Error detect */
> +	__be32	errdis;		/* 0xE44 - Error disable */
> +	__be32	errinten;	/* 0xE48 - Error interrupt enable */
> +	__be32	errattr;	/* 0xE4c - Error attribute capture */
> +	__be32	erradrrl;	/* 0xE50 - Error address capture low */
> +	__be32	erradrrh;	/* 0xE54 - Error address capture high */
> +	__be32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};

Should those macros and struct go in a .h file ?


> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);

What happens to that mapping once you leave the function ?

By the way, new drivers use platform_get_resource(ofdev, IORESOURCE_MEM, 0);

> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);

out_be32() and in_be32() are specific to powerpc, they should be avoided 
outside of arch/powerpc/

You have iowrite32be() and ioread32be() which are equivalent.

> +
> +	/* write bits[18-21] to srbare0 */
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT))
> +		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();

Why do you need eieio() after a setbits32() which already do a full sync ?

> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +	.access = generic_access_phys,
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +			       struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +	    (mem->size == 0) ||
> +	    (mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +			       vma->vm_start,
> +			       mem->addr >> PAGE_SHIFT,
> +			       vma->vm_end - vma->vm_start,
> +			       vma->vm_page_prot);
> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	u32 mem_base_32;
> +	u64 mem_base_64;

mem_base_32 and mem_base_64 needed here, define them inside the block 
where they are used, see below

> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	/* alloc uio_info for one device */
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	/* get optional uio name */
> +	if (of_property_read_string(node, "uio_name", &dt_name))
> +		dt_name = UIO_NAME;
> +
> +	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
> +	if (!info->name)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-size\n");
> +		return -EINVAL;
> +	}
> +
> +
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
		u64 mem_base_64;

> +		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
> +		mem_base = mem_base_64;
> +	} else {

		u32 mem_base_32;

> +		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
> +		mem_base = mem_base_32;
> +	}
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-base\n");
> +		return -EINVAL;
> +	}
> +
> +	if (mem_size == 0) {
> +		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing l2cache-size\n");
> +		return -EINVAL;
> +	}
> +
> +	rem = l2cache_size % mem_size;
> +	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
> +	if (rem || (ways & (ways - 1))) {

Use is_power_of_2() instead ?

> +		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = uio_cache_sram_setup(pdev, mem_base, ways);
> +	if (ret)
> +		return ret;
> +
> +	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
> +		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
> +		ret = -ENXIO;

ret is never used.

> +	}
> +
> +	info->irq = UIO_IRQ_NONE;
> +	info->version = UIO_INFO_VER;
> +	info->mmap = uio_cache_sram_mmap;
> +	uiomem = info->mem;
> +	uiomem->memtype = UIO_MEM_PHYS;
> +	uiomem->addr = mem_base;
> +	uiomem->size = mem_size;
> +	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
> +	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
> +	if (!uiomem->internal_addr) {
> +		dev_err(&pdev->dev, "cache ioremap failed\n");
> +		ret = -ENOMEM;

ret is never used.

> +	}
> +
> +	/* register uio device */
> +	if (uio_register_device(&pdev->dev, info)) {
> +		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
> +		ret = -ENODEV;
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(pdev, info);
> +	return 0;
> +
> +err_out:
> +	iounmap(info->mem[0].internal_addr);

What happends to the requested mem region ?

> +	return ret;
> +}
> +
> +static int uio_cache_sram_remove(struct platform_device *pdev)
> +{
> +	struct uio_info *info = platform_get_drvdata(pdev);
> +
> +	uio_unregister_device(info);
> +	iounmap(info->mem[0].internal_addr);

No release_mem_region() ?

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uio_cache_sram_of_match[] = {
> +	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> +	.probe = uio_cache_sram_probe,
> +	.remove = uio_cache_sram_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table	= uio_cache_sram_of_match,
> +	},
> +};
> +
> +module_platform_driver(uio_fsl_85xx_cache_sram);
> +
> +MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
> +MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-15  6:48           ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:48 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: linuxppc-dev, linux-kernel



Le 15/06/2022 à 07:57, Wang Wenhu a écrit :
> Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
> or fully. Users can make use of it as a block of independent memory that
> offers special usage, such as for debuging or other critical status info
> storage, which keeps consistently even when the whole system crashed.
> Applications can make use of UIO driver to access the SRAM from user level.
> 
> Once there was another driver version for the l2-cache-sram for SRAM access
> in kernel space. It had been removed recently.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
> v2:
>   - Use __be32 instead of u32 for big-endian data declarations;
>   - Use generic ioremap_cache instead of ioremap_coherent;
>   - Physical address support both 32 and 64 bits;
>   - Addressed some other comments from Greg.
> ---
>   drivers/uio/Kconfig                   |  14 ++
>   drivers/uio/Makefile                  |   1 +
>   drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
>   3 files changed, 303 insertions(+)
>   create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..f7604584a12c 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,20 @@ config UIO_NETX
>   	  To compile this driver as a module, choose M here; the module
>   	  will be called uio_netx.
>   
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Driver for user level access of freescale mpc85xx l2-cache-sram.
> +
> +	  Freescale's mpc85xx provides an option of configuring a part of
> +	  (or full) cache memory as SRAM. The driver does this configuring
> +	  work and exports SRAM to user-space for access form user level.
> +	  This is extremely helpful for user applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.
> +
>   config UIO_FSL_ELBC_GPCM
>   	tristate "eLBC/GPCM driver"
>   	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>   obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>   obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>   obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..6f91b0aa946b
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME	"uio_cache_sram"
> +
> +#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO		0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO		= 0,
> +	LOCK_WAYS_EIGHTH	= 1,
> +	LOCK_WAYS_TWO_EIGHTH	= 2,
> +	LOCK_WAYS_HALF		= 4,
> +	LOCK_WAYS_FULL		= 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	__be32	ctl;		/* 0x000 - L2 control */
> +	u8	res1[0xC];
> +	__be32	ewar0;		/* 0x010 - External write address 0 */
> +	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	__be32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	__be32	ewar1;		/* 0x020 - External write address 1 */
> +	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	__be32	ewar2;		/* 0x030 - External write address 2 */
> +	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	__be32	ewar3;		/* 0x040 - External write address 3 */
> +	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	__be32	captdatahi;	/* 0xE20 - Error data high capture */
> +	__be32	captdatalo;	/* 0xE24 - Error data low capture */
> +	__be32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	__be32	errdet;		/* 0xE40 - Error detect */
> +	__be32	errdis;		/* 0xE44 - Error disable */
> +	__be32	errinten;	/* 0xE48 - Error interrupt enable */
> +	__be32	errattr;	/* 0xE4c - Error attribute capture */
> +	__be32	erradrrl;	/* 0xE50 - Error address capture low */
> +	__be32	erradrrh;	/* 0xE54 - Error address capture high */
> +	__be32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};

Should those macros and struct go in a .h file ?


> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);

What happens to that mapping once you leave the function ?

By the way, new drivers use platform_get_resource(ofdev, IORESOURCE_MEM, 0);

> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);

out_be32() and in_be32() are specific to powerpc, they should be avoided 
outside of arch/powerpc/

You have iowrite32be() and ioread32be() which are equivalent.

> +
> +	/* write bits[18-21] to srbare0 */
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT))
> +		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();

Why do you need eieio() after a setbits32() which already do a full sync ?

> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +	.access = generic_access_phys,
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +			       struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +	    (mem->size == 0) ||
> +	    (mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +			       vma->vm_start,
> +			       mem->addr >> PAGE_SHIFT,
> +			       vma->vm_end - vma->vm_start,
> +			       vma->vm_page_prot);
> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	u32 mem_base_32;
> +	u64 mem_base_64;

mem_base_32 and mem_base_64 needed here, define them inside the block 
where they are used, see below

> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	/* alloc uio_info for one device */
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	/* get optional uio name */
> +	if (of_property_read_string(node, "uio_name", &dt_name))
> +		dt_name = UIO_NAME;
> +
> +	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
> +	if (!info->name)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-size\n");
> +		return -EINVAL;
> +	}
> +
> +
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
		u64 mem_base_64;

> +		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
> +		mem_base = mem_base_64;
> +	} else {

		u32 mem_base_32;

> +		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
> +		mem_base = mem_base_32;
> +	}
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-base\n");
> +		return -EINVAL;
> +	}
> +
> +	if (mem_size == 0) {
> +		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing l2cache-size\n");
> +		return -EINVAL;
> +	}
> +
> +	rem = l2cache_size % mem_size;
> +	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
> +	if (rem || (ways & (ways - 1))) {

Use is_power_of_2() instead ?

> +		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = uio_cache_sram_setup(pdev, mem_base, ways);
> +	if (ret)
> +		return ret;
> +
> +	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
> +		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
> +		ret = -ENXIO;

ret is never used.

> +	}
> +
> +	info->irq = UIO_IRQ_NONE;
> +	info->version = UIO_INFO_VER;
> +	info->mmap = uio_cache_sram_mmap;
> +	uiomem = info->mem;
> +	uiomem->memtype = UIO_MEM_PHYS;
> +	uiomem->addr = mem_base;
> +	uiomem->size = mem_size;
> +	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
> +	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
> +	if (!uiomem->internal_addr) {
> +		dev_err(&pdev->dev, "cache ioremap failed\n");
> +		ret = -ENOMEM;

ret is never used.

> +	}
> +
> +	/* register uio device */
> +	if (uio_register_device(&pdev->dev, info)) {
> +		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
> +		ret = -ENODEV;
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(pdev, info);
> +	return 0;
> +
> +err_out:
> +	iounmap(info->mem[0].internal_addr);

What happends to the requested mem region ?

> +	return ret;
> +}
> +
> +static int uio_cache_sram_remove(struct platform_device *pdev)
> +{
> +	struct uio_info *info = platform_get_drvdata(pdev);
> +
> +	uio_unregister_device(info);
> +	iounmap(info->mem[0].internal_addr);

No release_mem_region() ?

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uio_cache_sram_of_match[] = {
> +	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> +	.probe = uio_cache_sram_probe,
> +	.remove = uio_cache_sram_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table	= uio_cache_sram_of_match,
> +	},
> +};
> +
> +module_platform_driver(uio_fsl_85xx_cache_sram);
> +
> +MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
> +MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-15  6:49           ` Christoph Hellwig
  -1 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-15  6:49 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas, akpm,
	linux-fpga, linux-pci, linux-mm, christophe.leroy, linux-kernel,
	linuxppc-dev, mpe

Did you verify that all architectures actually provide a ioremap_prot
prototype?
The header situation for ioremap* is a mess unfortunately.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
@ 2022-06-15  6:49           ` Christoph Hellwig
  0 siblings, 0 replies; 52+ messages in thread
From: Christoph Hellwig @ 2022-06-15  6:49 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: arnd, gregkh, linux-fpga, linux-pci, linux-kernel, linux-mm, mdf,
	trix, bhelgaas, akpm, linuxppc-dev, yilun.xu, hao.wu

Did you verify that all architectures actually provide a ioremap_prot
prototype?
The header situation for ioremap* is a mess unfortunately.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-15  6:49           ` Christophe Leroy
  -1 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:49 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: linux-kernel, linuxppc-dev, mpe



Le 15/06/2022 à 07:57, Wang Wenhu a écrit :
> Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
> or fully. Users can make use of it as a block of independent memory that
> offers special usage, such as for debuging or other critical status info
> storage, which keeps consistently even when the whole system crashed.
> Applications can make use of UIO driver to access the SRAM from user level.
> 
> Once there was another driver version for the l2-cache-sram for SRAM access
> in kernel space. It had been removed recently.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
> v2:
>   - Use __be32 instead of u32 for big-endian data declarations;

I get the following warnings which 'make 
drivers/uio/uio_fsl_85xx_cache_sram.o C=2'

   CHECK   drivers/uio/uio_fsl_85xx_cache_sram.c
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    got restricted __be32 
[noderef] __iomem *


>   - Use generic ioremap_cache instead of ioremap_coherent;
>   - Physical address support both 32 and 64 bits;
>   - Addressed some other comments from Greg.
> ---
>   drivers/uio/Kconfig                   |  14 ++
>   drivers/uio/Makefile                  |   1 +
>   drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
>   3 files changed, 303 insertions(+)
>   create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..f7604584a12c 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,20 @@ config UIO_NETX
>   	  To compile this driver as a module, choose M here; the module
>   	  will be called uio_netx.
>   
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Driver for user level access of freescale mpc85xx l2-cache-sram.
> +
> +	  Freescale's mpc85xx provides an option of configuring a part of
> +	  (or full) cache memory as SRAM. The driver does this configuring
> +	  work and exports SRAM to user-space for access form user level.
> +	  This is extremely helpful for user applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.
> +
>   config UIO_FSL_ELBC_GPCM
>   	tristate "eLBC/GPCM driver"
>   	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>   obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>   obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>   obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..6f91b0aa946b
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME	"uio_cache_sram"
> +
> +#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO		0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO		= 0,
> +	LOCK_WAYS_EIGHTH	= 1,
> +	LOCK_WAYS_TWO_EIGHTH	= 2,
> +	LOCK_WAYS_HALF		= 4,
> +	LOCK_WAYS_FULL		= 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	__be32	ctl;		/* 0x000 - L2 control */
> +	u8	res1[0xC];
> +	__be32	ewar0;		/* 0x010 - External write address 0 */
> +	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	__be32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	__be32	ewar1;		/* 0x020 - External write address 1 */
> +	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	__be32	ewar2;		/* 0x030 - External write address 2 */
> +	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	__be32	ewar3;		/* 0x040 - External write address 3 */
> +	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	__be32	captdatahi;	/* 0xE20 - Error data high capture */
> +	__be32	captdatalo;	/* 0xE24 - Error data low capture */
> +	__be32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	__be32	errdet;		/* 0xE40 - Error detect */
> +	__be32	errdis;		/* 0xE44 - Error disable */
> +	__be32	errinten;	/* 0xE48 - Error interrupt enable */
> +	__be32	errattr;	/* 0xE4c - Error attribute capture */
> +	__be32	erradrrl;	/* 0xE50 - Error address capture low */
> +	__be32	erradrrh;	/* 0xE54 - Error address capture high */
> +	__be32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};
> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
> +
> +	/* write bits[18-21] to srbare0 */
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT))
> +		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();
> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +	.access = generic_access_phys,
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +			       struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +	    (mem->size == 0) ||
> +	    (mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +			       vma->vm_start,
> +			       mem->addr >> PAGE_SHIFT,
> +			       vma->vm_end - vma->vm_start,
> +			       vma->vm_page_prot);
> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	u32 mem_base_32;
> +	u64 mem_base_64;
> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	/* alloc uio_info for one device */
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	/* get optional uio name */
> +	if (of_property_read_string(node, "uio_name", &dt_name))
> +		dt_name = UIO_NAME;
> +
> +	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
> +	if (!info->name)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-size\n");
> +		return -EINVAL;
> +	}
> +
> +
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
> +		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
> +		mem_base = mem_base_64;
> +	} else {
> +		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
> +		mem_base = mem_base_32;
> +	}
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-base\n");
> +		return -EINVAL;
> +	}
> +
> +	if (mem_size == 0) {
> +		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing l2cache-size\n");
> +		return -EINVAL;
> +	}
> +
> +	rem = l2cache_size % mem_size;
> +	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
> +	if (rem || (ways & (ways - 1))) {
> +		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = uio_cache_sram_setup(pdev, mem_base, ways);
> +	if (ret)
> +		return ret;
> +
> +	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
> +		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
> +		ret = -ENXIO;
> +	}
> +
> +	info->irq = UIO_IRQ_NONE;
> +	info->version = UIO_INFO_VER;
> +	info->mmap = uio_cache_sram_mmap;
> +	uiomem = info->mem;
> +	uiomem->memtype = UIO_MEM_PHYS;
> +	uiomem->addr = mem_base;
> +	uiomem->size = mem_size;
> +	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
> +	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
> +	if (!uiomem->internal_addr) {
> +		dev_err(&pdev->dev, "cache ioremap failed\n");
> +		ret = -ENOMEM;
> +	}
> +
> +	/* register uio device */
> +	if (uio_register_device(&pdev->dev, info)) {
> +		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
> +		ret = -ENODEV;
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(pdev, info);
> +	return 0;
> +
> +err_out:
> +	iounmap(info->mem[0].internal_addr);
> +	return ret;
> +}
> +
> +static int uio_cache_sram_remove(struct platform_device *pdev)
> +{
> +	struct uio_info *info = platform_get_drvdata(pdev);
> +
> +	uio_unregister_device(info);
> +	iounmap(info->mem[0].internal_addr);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uio_cache_sram_of_match[] = {
> +	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> +	.probe = uio_cache_sram_probe,
> +	.remove = uio_cache_sram_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table	= uio_cache_sram_of_match,
> +	},
> +};
> +
> +module_platform_driver(uio_fsl_85xx_cache_sram);
> +
> +MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
> +MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
@ 2022-06-15  6:49           ` Christophe Leroy
  0 siblings, 0 replies; 52+ messages in thread
From: Christophe Leroy @ 2022-06-15  6:49 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: linuxppc-dev, linux-kernel



Le 15/06/2022 à 07:57, Wang Wenhu a écrit :
> Freescale mpc85xx l2-cache could be optionally configured as SRAM partly
> or fully. Users can make use of it as a block of independent memory that
> offers special usage, such as for debuging or other critical status info
> storage, which keeps consistently even when the whole system crashed.
> Applications can make use of UIO driver to access the SRAM from user level.
> 
> Once there was another driver version for the l2-cache-sram for SRAM access
> in kernel space. It had been removed recently.
> See: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> 
> Signed-off-by: Wang Wenhu <wenhu.wang@hotmail.com>
> ---
> v2:
>   - Use __be32 instead of u32 for big-endian data declarations;

I get the following warnings which 'make 
drivers/uio/uio_fsl_85xx_cache_sram.o C=2'

   CHECK   drivers/uio/uio_fsl_85xx_cache_sram.c
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:96:19:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:100:27:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:102:9:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:106:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:110:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:114:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    expected unsigned int 
volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    got restricted __be32 
[noderef] __iomem *
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17: warning: incorrect type in 
argument 1 (different base types)
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    expected unsigned int 
const volatile [noderef] [usertype] __iomem *addr
drivers/uio/uio_fsl_85xx_cache_sram.c:119:17:    got restricted __be32 
[noderef] __iomem *


>   - Use generic ioremap_cache instead of ioremap_coherent;
>   - Physical address support both 32 and 64 bits;
>   - Addressed some other comments from Greg.
> ---
>   drivers/uio/Kconfig                   |  14 ++
>   drivers/uio/Makefile                  |   1 +
>   drivers/uio/uio_fsl_85xx_cache_sram.c | 288 ++++++++++++++++++++++++++
>   3 files changed, 303 insertions(+)
>   create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> 
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 2e16c5338e5b..f7604584a12c 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -105,6 +105,20 @@ config UIO_NETX
>   	  To compile this driver as a module, choose M here; the module
>   	  will be called uio_netx.
>   
> +config UIO_FSL_85XX_CACHE_SRAM
> +	tristate "Freescale 85xx L2-Cache-SRAM UIO driver"
> +	depends on FSL_SOC_BOOKE && PPC32
> +	help
> +	  Driver for user level access of freescale mpc85xx l2-cache-sram.
> +
> +	  Freescale's mpc85xx provides an option of configuring a part of
> +	  (or full) cache memory as SRAM. The driver does this configuring
> +	  work and exports SRAM to user-space for access form user level.
> +	  This is extremely helpful for user applications that require
> +	  high performance memory accesses.
> +
> +	  If you don't know what to do here, say N.
> +
>   config UIO_FSL_ELBC_GPCM
>   	tristate "eLBC/GPCM driver"
>   	depends on FSL_LBC
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index f2f416a14228..1ba07d92a1b1 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
>   obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
>   obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
>   obj-$(CONFIG_UIO_DFL)	+= uio_dfl.o
> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM)	+= uio_fsl_85xx_cache_sram.o
> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c b/drivers/uio/uio_fsl_85xx_cache_sram.c
> new file mode 100644
> index 000000000000..6f91b0aa946b
> --- /dev/null
> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Wang Wenhu <wenhu.wang@hotmail.com>
> + * All rights reserved.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/stringify.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +
> +#define DRIVER_NAME	"uio_mpc85xx_cache_sram"
> +#define UIO_INFO_VER	"0.0.1"
> +#define UIO_NAME	"uio_cache_sram"
> +
> +#define L2CR_L2FI		0x40000000	/* L2 flash invalidate */
> +#define L2CR_L2IO		0x00200000	/* L2 instruction only */
> +#define L2CR_SRAM_ZERO		0x00000000	/* L2SRAM zero size */
> +#define L2CR_SRAM_FULL		0x00010000	/* L2SRAM full size */
> +#define L2CR_SRAM_HALF		0x00020000	/* L2SRAM half size */
> +#define L2CR_SRAM_TWO_HALFS	0x00030000	/* L2SRAM two half sizes */
> +#define L2CR_SRAM_QUART		0x00040000	/* L2SRAM one quarter size */
> +#define L2CR_SRAM_TWO_QUARTS	0x00050000	/* L2SRAM two quarter size */
> +#define L2CR_SRAM_EIGHTH	0x00060000	/* L2SRAM one eighth size */
> +#define L2CR_SRAM_TWO_EIGHTH	0x00070000	/* L2SRAM two eighth size */
> +
> +#define L2SRAM_OPTIMAL_SZ_SHIFT	0x00000003	/* Optimum size for L2SRAM */
> +
> +#define L2SRAM_BAR_MSK_LO18	0xFFFFC000	/* Lower 18 bits */
> +#define L2SRAM_BARE_MSK_HI4	0x0000000F	/* Upper 4 bits */
> +
> +enum cache_sram_lock_ways {
> +	LOCK_WAYS_ZERO		= 0,
> +	LOCK_WAYS_EIGHTH	= 1,
> +	LOCK_WAYS_TWO_EIGHTH	= 2,
> +	LOCK_WAYS_HALF		= 4,
> +	LOCK_WAYS_FULL		= 8,
> +};
> +
> +struct mpc85xx_l2ctlr {
> +	__be32	ctl;		/* 0x000 - L2 control */
> +	u8	res1[0xC];
> +	__be32	ewar0;		/* 0x010 - External write address 0 */
> +	__be32	ewarea0;	/* 0x014 - External write address extended 0 */
> +	__be32	ewcr0;		/* 0x018 - External write ctrl */
> +	u8	res2[4];
> +	__be32	ewar1;		/* 0x020 - External write address 1 */
> +	__be32	ewarea1;	/* 0x024 - External write address extended 1 */
> +	__be32	ewcr1;		/* 0x028 - External write ctrl 1 */
> +	u8	res3[4];
> +	__be32	ewar2;		/* 0x030 - External write address 2 */
> +	__be32	ewarea2;	/* 0x034 - External write address extended 2 */
> +	__be32	ewcr2;		/* 0x038 - External write ctrl 2 */
> +	u8	res4[4];
> +	__be32	ewar3;		/* 0x040 - External write address 3 */
> +	__be32	ewarea3;	/* 0x044 - External write address extended 3 */
> +	__be32	ewcr3;		/* 0x048 - External write ctrl 3 */
> +	u8	res5[0xB4];
> +	__be32	srbar0;		/* 0x100 - SRAM base address 0 */
> +	__be32	srbarea0;	/* 0x104 - SRAM base addr reg ext address 0 */
> +	__be32	srbar1;		/* 0x108 - SRAM base address 1 */
> +	__be32	srbarea1;	/* 0x10C - SRAM base addr reg ext address 1 */
> +	u8	res6[0xCF0];
> +	__be32	errinjhi;	/* 0xE00 - Error injection mask high */
> +	__be32	errinjlo;	/* 0xE04 - Error injection mask low */
> +	__be32	errinjctl;	/* 0xE08 - Error injection tag/ecc control */
> +	u8	res7[0x14];
> +	__be32	captdatahi;	/* 0xE20 - Error data high capture */
> +	__be32	captdatalo;	/* 0xE24 - Error data low capture */
> +	__be32	captecc;	/* 0xE28 - Error syndrome */
> +	u8	res8[0x14];
> +	__be32	errdet;		/* 0xE40 - Error detect */
> +	__be32	errdis;		/* 0xE44 - Error disable */
> +	__be32	errinten;	/* 0xE48 - Error interrupt enable */
> +	__be32	errattr;	/* 0xE4c - Error attribute capture */
> +	__be32	erradrrl;	/* 0xE50 - Error address capture low */
> +	__be32	erradrrh;	/* 0xE54 - Error address capture high */
> +	__be32	errctl;		/* 0xE58 - Error control */
> +	u8	res9[0x1A4];
> +};
> +
> +static int uio_cache_sram_setup(struct platform_device *pdev,
> +				phys_addr_t base, u8 ways)
> +{
> +	struct mpc85xx_l2ctlr __iomem *l2ctlr = of_iomap(pdev->dev.of_node, 0);
> +
> +	if (!l2ctlr) {
> +		dev_err(&pdev->dev, "can not map l2 controller\n");
> +		return -EINVAL;
> +	}
> +
> +	/* write bits[0-17] to srbar0 */
> +	out_be32(&l2ctlr->srbar0, lower_32_bits(base) & L2SRAM_BAR_MSK_LO18);
> +
> +	/* write bits[18-21] to srbare0 */
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT))
> +		out_be32(&l2ctlr->srbarea0, upper_32_bits(base) & L2SRAM_BARE_MSK_HI4);
> +
> +	clrsetbits_be32(&l2ctlr->ctl, L2CR_L2E, L2CR_L2FI);
> +
> +	switch (ways) {
> +	case LOCK_WAYS_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
> +		break;
> +
> +	case LOCK_WAYS_TWO_EIGHTH:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
> +		break;
> +
> +	case LOCK_WAYS_HALF:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
> +		break;
> +
> +	case LOCK_WAYS_FULL:
> +	default:
> +		setbits32(&l2ctlr->ctl, L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
> +		break;
> +	}
> +	eieio();
> +
> +	return 0;
> +}
> +
> +static const struct vm_operations_struct uio_cache_sram_vm_ops = {
> +	.access = generic_access_phys,
> +};
> +
> +static int uio_cache_sram_mmap(struct uio_info *info,
> +			       struct vm_area_struct *vma)
> +{
> +	struct uio_mem *mem = info->mem;
> +
> +	if (mem->addr & ~PAGE_MASK)
> +		return -ENODEV;
> +
> +	if ((vma->vm_end - vma->vm_start > mem->size) ||
> +	    (mem->size == 0) ||
> +	    (mem->memtype != UIO_MEM_PHYS))
> +		return -EINVAL;
> +
> +	vma->vm_ops = &uio_cache_sram_vm_ops;
> +	vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> +	return remap_pfn_range(vma,
> +			       vma->vm_start,
> +			       mem->addr >> PAGE_SHIFT,
> +			       vma->vm_end - vma->vm_start,
> +			       vma->vm_page_prot);
> +}
> +
> +static int uio_cache_sram_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct uio_info *info;
> +	struct uio_mem *uiomem;
> +	const char *dt_name;
> +	u32 mem_base_32;
> +	u64 mem_base_64;
> +	phys_addr_t mem_base;
> +	u32 l2cache_size;
> +	u32 mem_size;
> +	u32 rem;
> +	u8 ways;
> +	int ret;
> +
> +	/* alloc uio_info for one device */
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	/* get optional uio name */
> +	if (of_property_read_string(node, "uio_name", &dt_name))
> +		dt_name = UIO_NAME;
> +
> +	info->name = devm_kstrdup(&pdev->dev, dt_name, GFP_KERNEL);
> +	if (!info->name)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(node, "cache-mem-size", &mem_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-size\n");
> +		return -EINVAL;
> +	}
> +
> +
> +	if (IS_ENABLED(CONFIG_PHYS_64BIT)) {
> +		ret = of_property_read_u64(node, "cache-mem-base", &mem_base_64);
> +		mem_base = mem_base_64;
> +	} else {
> +		ret = of_property_read_u32(node, "cache-mem-base", &mem_base_32);
> +		mem_base = mem_base_32;
> +	}
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing cache-mem-base\n");
> +		return -EINVAL;
> +	}
> +
> +	if (mem_size == 0) {
> +		dev_err(&pdev->dev, "cache-mem-size should not be 0\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(node, "cache-size", &l2cache_size);
> +	if (ret) {
> +		dev_err(&pdev->dev, "missing l2cache-size\n");
> +		return -EINVAL;
> +	}
> +
> +	rem = l2cache_size % mem_size;
> +	ways = LOCK_WAYS_FULL * mem_size / l2cache_size;
> +	if (rem || (ways & (ways - 1))) {
> +		dev_err(&pdev->dev, "illegal cache-sram-size parameter\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = uio_cache_sram_setup(pdev, mem_base, ways);
> +	if (ret)
> +		return ret;
> +
> +	if (!request_mem_region(mem_base, mem_size, "fsl_85xx_cache_sram")) {
> +		dev_err(&pdev->dev, "uio_cache_sram request memory failed\n");
> +		ret = -ENXIO;
> +	}
> +
> +	info->irq = UIO_IRQ_NONE;
> +	info->version = UIO_INFO_VER;
> +	info->mmap = uio_cache_sram_mmap;
> +	uiomem = info->mem;
> +	uiomem->memtype = UIO_MEM_PHYS;
> +	uiomem->addr = mem_base;
> +	uiomem->size = mem_size;
> +	uiomem->name = devm_kstrdup(&pdev->dev, node->name, GFP_KERNEL);
> +	uiomem->internal_addr = ioremap_cache(mem_base, mem_size);
> +	if (!uiomem->internal_addr) {
> +		dev_err(&pdev->dev, "cache ioremap failed\n");
> +		ret = -ENOMEM;
> +	}
> +
> +	/* register uio device */
> +	if (uio_register_device(&pdev->dev, info)) {
> +		dev_err(&pdev->dev, "error uio,cache-sram registration failed\n");
> +		ret = -ENODEV;
> +		goto err_out;
> +	}
> +
> +	platform_set_drvdata(pdev, info);
> +	return 0;
> +
> +err_out:
> +	iounmap(info->mem[0].internal_addr);
> +	return ret;
> +}
> +
> +static int uio_cache_sram_remove(struct platform_device *pdev)
> +{
> +	struct uio_info *info = platform_get_drvdata(pdev);
> +
> +	uio_unregister_device(info);
> +	iounmap(info->mem[0].internal_addr);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uio_cache_sram_of_match[] = {
> +	{ .compatible = "fsl,p2020-l2-cache-sram-uio", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, uio_cache_sram_of_match);
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> +	.probe = uio_cache_sram_probe,
> +	.remove = uio_cache_sram_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table	= uio_cache_sram_of_match,
> +	},
> +};
> +
> +module_platform_driver(uio_fsl_85xx_cache_sram);
> +
> +MODULE_AUTHOR("Wang Wenhu <wenhu.wang@hotmail.com>");
> +MODULE_DESCRIPTION("Freescale MPC85xx Cache-SRAM UIO Platform Driver");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-22  2:22           ` kernel test robot
  -1 siblings, 0 replies; 52+ messages in thread
From: kernel test robot @ 2022-06-22  2:22 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: kbuild-all, christophe.leroy, linux-kernel, linuxppc-dev, mpe,
	wenhu.wang

Hi Wang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: arm-randconfig-s032-20220622 (https://download.01.org/0day-ci/archive/20220622/202206221053.GV7BYwqL-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-31-g4880bd19-dirty
        # https://github.com/intel-lab-lkp/linux/commit/b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
        git checkout b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arm-linux-gnueabi-ld: drivers/char/mem.o:(.rodata+0x144): undefined reference to `generic_access_phys'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
@ 2022-06-22  2:22           ` kernel test robot
  0 siblings, 0 replies; 52+ messages in thread
From: kernel test robot @ 2022-06-22  2:22 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: kbuild-all, wenhu.wang, linux-kernel, linuxppc-dev

Hi Wang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: arm-randconfig-s032-20220622 (https://download.01.org/0day-ci/archive/20220622/202206221053.GV7BYwqL-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-31-g4880bd19-dirty
        # https://github.com/intel-lab-lkp/linux/commit/b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
        git checkout b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arm-linux-gnueabi-ld: drivers/char/mem.o:(.rodata+0x144): undefined reference to `generic_access_phys'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-15  5:57         ` Wang Wenhu
@ 2022-06-22  4:13           ` kernel test robot
  -1 siblings, 0 replies; 52+ messages in thread
From: kernel test robot @ 2022-06-22  4:13 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: kbuild-all, christophe.leroy, linux-kernel, linuxppc-dev, mpe,
	wenhu.wang

Hi Wang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: m68k-buildonly-randconfig-r001-20220622 (https://download.01.org/0day-ci/archive/20220622/202206221102.w7hylFXN-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
        git checkout b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> m68k-linux-ld: drivers/char/mem.o:(.rodata+0x37c): undefined reference to `generic_access_phys'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
@ 2022-06-22  4:13           ` kernel test robot
  0 siblings, 0 replies; 52+ messages in thread
From: kernel test robot @ 2022-06-22  4:13 UTC (permalink / raw)
  To: Wang Wenhu, gregkh, arnd, hao.wu, trix, mdf, yilun.xu, bhelgaas,
	akpm, linux-fpga, linux-pci, linux-mm
  Cc: kbuild-all, wenhu.wang, linux-kernel, linuxppc-dev

Hi Wang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: m68k-buildonly-randconfig-r001-20220622 (https://download.01.org/0day-ci/archive/20220622/202206221102.w7hylFXN-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Wang-Wenhu/mm-eliminate-ifdef-of-HAVE_IOREMAP_PROT-in-c-files/20220615-140135
        git checkout b20efcc877829b6f416cf111bd5ad2b13a0cd08e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> m68k-linux-ld: drivers/char/mem.o:(.rodata+0x37c): undefined reference to `generic_access_phys'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2022-06-22  4:14 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220609102855.272270-1-wenhu.wang@hotmail.com>
2022-06-09 10:28 ` [PATCH 1/2] powerpc:mm: export symbol ioremap_coherent Wang Wenhu
2022-06-09 10:28   ` Wang Wenhu
2022-06-14 10:45   ` Michael Ellerman
2022-06-14 14:49     ` Christoph Hellwig
2022-06-14 14:49       ` Christoph Hellwig
2022-06-15  4:37     ` 回复: " Wenhu Wang
2022-06-09 10:28 ` [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation Wang Wenhu
2022-06-09 10:28   ` Wang Wenhu
2022-06-09 13:17   ` Greg KH
2022-06-09 13:17     ` Greg KH
2022-06-14  6:09     ` 回复: " Wenhu Wang
2022-06-14  6:09       ` Wenhu Wang
2022-06-14  6:34       ` Greg KH
2022-06-14  6:34         ` Greg KH
2022-06-14  7:12         ` Christophe Leroy
2022-06-14  7:12           ` Christophe Leroy
2022-06-14  7:18       ` Christophe Leroy
2022-06-14  7:18         ` Christophe Leroy
2022-06-14  7:26         ` Christophe Leroy
2022-06-14  7:26           ` Christophe Leroy
2022-06-14  7:53           ` Wenhu Wang
2022-06-14  7:53             ` Wenhu Wang
2022-06-14  8:03             ` Greg KH
2022-06-14  8:03               ` Greg KH
2022-06-14  8:39               ` Wenhu Wang
2022-06-14  8:39                 ` Wenhu Wang
2022-06-14 14:40             ` Wenhu Wang
2022-06-14 14:40               ` Wenhu Wang
2022-06-14 14:59               ` Christophe Leroy
2022-06-14 14:59                 ` Christophe Leroy
2022-06-15  6:04       ` 回复: " Christophe Leroy
2022-06-15  6:04         ` Christophe Leroy
2022-06-15  5:57     ` [PATCHv2 0/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver Wang Wenhu
2022-06-15  5:57       ` Wang Wenhu
     [not found]     ` <20220615055735.53585-1-wenhu.wang@hotmail.com>
2022-06-15  5:57       ` [PATCHv2 1/2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files Wang Wenhu
2022-06-15  5:57         ` Wang Wenhu
2022-06-15  6:49         ` Christoph Hellwig
2022-06-15  6:49           ` Christoph Hellwig
2022-06-22  2:22         ` kernel test robot
2022-06-22  2:22           ` kernel test robot
2022-06-22  4:13         ` kernel test robot
2022-06-22  4:13           ` kernel test robot
2022-06-15  5:57       ` [PATCHv2 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation Wang Wenhu
2022-06-15  5:57         ` Wang Wenhu
2022-06-15  6:43         ` Christoph Hellwig
2022-06-15  6:43           ` Christoph Hellwig
2022-06-15  6:48         ` Christophe Leroy
2022-06-15  6:48           ` Christophe Leroy
2022-06-15  6:49         ` Christophe Leroy
2022-06-15  6:49           ` Christophe Leroy
2022-06-14 14:51   ` [PATCH " Christoph Hellwig
2022-06-14 14:51     ` Christoph Hellwig

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.