Re: [PATCH RFC v3 2/2] memory: pl353: Add driver for arm pl353 static memory controller
On Friday 27 June 2014 08:32:42 Punnaiah Choudary Kalluri wrote:
> +/**
> + * struct pl353_smc_data - Private smc driver structure
> + * @devclk: Pointer to the peripheral clock
> + * @aperclk: Pointer to the APER clock
> + */
> +struct pl353_smc_data {
> + struct clk *memclk;
> + struct clk *aclk;
> +};
> +
> +/* SMC virtual register base */
> +static void __iomem *pl353_smc_base;
Drivers should not assume that there is only a single instance of the
hardware in the system. Please remove this global variable and
change the code to pass around a pointer to a structure containing
it.
> +/**
> + * pl353_smc_set_buswidth - Set memory buswidth
> + * @bw: Memory buswidth (8 | 16)
> + * Return: 0 on success or negative errno.
> + */
> +int pl353_smc_set_buswidth(unsigned int bw)
> +{
> +
> + if (bw != PL353_SMC_MEM_WIDTH_8 && bw != PL353_SMC_MEM_WIDTH_16)
> + return -EINVAL;
> +
> + writel(bw, pl353_smc_base + PL353_SMC_SET_OPMODE_OFFS);
> + writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
> +PL353_SMC_DIRECT_CMD_OFFS);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pl353_smc_set_buswidth);
Why is this an exported interface? Wouldn't that setting just be
determined from DT when the device is probed?
> +/**
> + * pl353_smc_get_nand_int_status_raw - Get NAND interrupt status bit
> + * Return: the raw_int_status1 bit from the memc_status register
> + */
> +int pl353_smc_get_nand_int_status_raw(void)
> +{
> + u32 reg;
> +
> + reg = readl(pl353_smc_base + PL353_SMC_MEMC_STATUS_OFFS);
> + reg >>= PL353_SMC_MEMC_STATUS_RAW_INT_1_SHIFT;
> + reg &= 1;
> +
> + return reg;
> +}
> +EXPORT_SYMBOL_GPL(pl353_smc_get_nand_int_status_raw);
> +
> +/**
> + * pl353_smc_clr_nand_int - Clear NAND interrupt
> + */
> +void pl353_smc_clr_nand_int(void)
> +{
> + writel(PL353_SMC_CFG_CLR_INT_CLR_1,
> + pl353_smc_base + PL353_SMC_CFG_CLR_OFFS);
> +}
> +EXPORT_SYMBOL_GPL(pl353_smc_clr_nand_int);
Why aren't these just part of the NAND driver?
> + pl353_smc_set_cycles(t_rc, t_wc, t_rea, t_wp, t_clr, t_ar, t_rr);
> + writel(PL353_SMC_CFG_CLR_INT_CLR_1,
> + pl353_smc_base + PL353_SMC_CFG_CLR_OFFS);
> + writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
> +PL353_SMC_DIRECT_CMD_OFFS);
> + /* Wait till the ECC operation is complete */
> + do {
> + if (pl353_smc_ecc_is_busy_noirq())
> + cpu_relax();
> + else
> + break;
> + } while (!time_after_eq(jiffies, timeout));
This blocks the CPU for up to a second (the timeout value). Since you are
not in atomic context, you can instead sleep here, e.g. using msleep(1)
or some other appropriate timeout.
What is the expected average duration of the loop?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v3 2/2] memory: pl353: Add driver for arm pl353 static memory controller
On Friday 27 June 2014 08:32:42 Punnaiah Choudary Kalluri wrote:
+/**
+ * struct pl353_smc_data - Private smc driver structure
+ * @devclk: Pointer to the peripheral clock
+ * @aperclk: Pointer to the APER clock
+ */
+struct pl353_smc_data {
+ struct clk *memclk;
+ struct clk *aclk;
+};
+
+/* SMC virtual register base */
+static void __iomem *pl353_smc_base;
Drivers should not assume that there is only a single instance of the
hardware in the system. Please remove this global variable and
change the code to pass around a pointer to a structure containing
it.
+/**
+ * pl353_smc_set_buswidth - Set memory buswidth
+ * @bw: Memory buswidth (8 | 16)
+ * Return: 0 on success or negative errno.
+ */
+int pl353_smc_set_buswidth(unsigned int bw)
+{
+
+ if (bw != PL353_SMC_MEM_WIDTH_8 bw != PL353_SMC_MEM_WIDTH_16)
+ return -EINVAL;
+
+ writel(bw, pl353_smc_base + PL353_SMC_SET_OPMODE_OFFS);
+ writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
+PL353_SMC_DIRECT_CMD_OFFS);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pl353_smc_set_buswidth);
Why is this an exported interface? Wouldn't that setting just be
determined from DT when the device is probed?
+/**
+ * pl353_smc_get_nand_int_status_raw - Get NAND interrupt status bit
+ * Return: the raw_int_status1 bit from the memc_status register
+ */
+int pl353_smc_get_nand_int_status_raw(void)
+{
+ u32 reg;
+
+ reg = readl(pl353_smc_base + PL353_SMC_MEMC_STATUS_OFFS);
+ reg = PL353_SMC_MEMC_STATUS_RAW_INT_1_SHIFT;
+ reg = 1;
+
+ return reg;
+}
+EXPORT_SYMBOL_GPL(pl353_smc_get_nand_int_status_raw);
+
+/**
+ * pl353_smc_clr_nand_int - Clear NAND interrupt
+ */
+void pl353_smc_clr_nand_int(void)
+{
+ writel(PL353_SMC_CFG_CLR_INT_CLR_1,
+ pl353_smc_base + PL353_SMC_CFG_CLR_OFFS);
+}
+EXPORT_SYMBOL_GPL(pl353_smc_clr_nand_int);
Why aren't these just part of the NAND driver?
+ pl353_smc_set_cycles(t_rc, t_wc, t_rea, t_wp, t_clr, t_ar, t_rr);
+ writel(PL353_SMC_CFG_CLR_INT_CLR_1,
+ pl353_smc_base + PL353_SMC_CFG_CLR_OFFS);
+ writel(PL353_SMC_DC_UPT_NAND_REGS, pl353_smc_base +
+PL353_SMC_DIRECT_CMD_OFFS);
+ /* Wait till the ECC operation is complete */
+ do {
+ if (pl353_smc_ecc_is_busy_noirq())
+ cpu_relax();
+ else
+ break;
+ } while (!time_after_eq(jiffies, timeout));
This blocks the CPU for up to a second (the timeout value). Since you are
not in atomic context, you can instead sleep here, e.g. using msleep(1)
or some other appropriate timeout.
What is the expected average duration of the loop?
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC v3 2/2] memory: pl353: Add driver for arm pl353 static memory controller
Add driver for arm pl353 static memory controller. This controller is used in xilinx zynq soc for interfacing the nand and nor/sram memory devices. Signed-off-by: Punnaiah Choudary Kalluri --- Changes in v2: - Since now the timing parameters are in nano seconds, added logic to convert them to the cycles --- drivers/memory/Kconfig |6 + drivers/memory/Makefile |1 + drivers/memory/pl353-smc.c | 569 ++ include/linux/memory/pl353-smc.h | 34 +++ 4 files changed, 610 insertions(+), 0 deletions(-) create mode 100644 drivers/memory/pl353-smc.c create mode 100644 include/linux/memory/pl353-smc.h diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig index c59e9c9..58c29a2 100644 --- a/drivers/memory/Kconfig +++ b/drivers/memory/Kconfig @@ -65,4 +65,10 @@ config FSL_IFC bool depends on FSL_SOC +config PL353_SMC + bool "ARM PL353 Static Memory Controller(SMC) driver" + default y + depends on ARM + help + This driver is for the ARM PL353 Static Memory Controller(SMC) module. endif diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index 71160a2..5040977 100644 --- a/drivers/memory/Makefile +++ b/drivers/memory/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_IFC) += fsl_ifc.o obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o +obj-$(CONFIG_PL353_SMC)+= pl353-smc.o diff --git a/drivers/memory/pl353-smc.c b/drivers/memory/pl353-smc.c new file mode 100644 index 000..02221ba --- /dev/null +++ b/drivers/memory/pl353-smc.c @@ -0,0 +1,569 @@ +/* + * ARM PL353 SMC Driver + * + * Copyright (C) 2012 - 2014 Xilinx, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Currently only a single SMC instance is supported. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* Register definitions */ +#define PL353_SMC_MEMC_STATUS_OFFS 0 /* Controller status reg, RO */ +#define PL353_SMC_CFG_CLR_OFFS 0xC /* Clear config reg, WO */ +#define PL353_SMC_DIRECT_CMD_OFFS 0x10/* Direct command reg, WO */ +#define PL353_SMC_SET_CYCLES_OFFS 0x14/* Set cycles register, WO */ +#define PL353_SMC_SET_OPMODE_OFFS 0x18/* Set opmode register, WO */ +#define PL353_SMC_ECC_STATUS_OFFS 0x400 /* ECC status register */ +#define PL353_SMC_ECC_MEMCFG_OFFS 0x404 /* ECC mem config reg */ +#define PL353_SMC_ECC_MEMCMD1_OFFS 0x408 /* ECC mem cmd1 reg */ +#define PL353_SMC_ECC_MEMCMD2_OFFS 0x40C /* ECC mem cmd2 reg */ +#define PL353_SMC_ECC_VALUE0_OFFS 0x418 /* ECC value 0 reg */ + +/* Controller status register specifc constants */ +#define PL353_SMC_MEMC_STATUS_RAW_INT_1_SHIFT 6 + +/* Clear configuration register specific constants */ +#define PL353_SMC_CFG_CLR_INT_CLR_10x10 +#define PL353_SMC_CFG_CLR_ECC_INT_DIS_10x40 +#define PL353_SMC_CFG_CLR_INT_DIS_10x2 +#define PL353_SMC_CFG_CLR_DEFAULT_MASK (PL353_SMC_CFG_CLR_INT_CLR_1 | \ +PL353_SMC_CFG_CLR_ECC_INT_DIS_1 | \ +PL353_SMC_CFG_CLR_INT_DIS_1) + +/* Set cycles register specific constants */ +#define PL353_SMC_SET_CYCLES_T0_MASK 0xF +#define PL353_SMC_SET_CYCLES_T0_SHIFT 0 +#define PL353_SMC_SET_CYCLES_T1_MASK 0xF +#define PL353_SMC_SET_CYCLES_T1_SHIFT 4 +#define PL353_SMC_SET_CYCLES_T2_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T2_SHIFT 8 +#define PL353_SMC_SET_CYCLES_T3_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T3_SHIFT 11 +#define PL353_SMC_SET_CYCLES_T4_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T4_SHIFT 14 +#define PL353_SMC_SET_CYCLES_T5_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T5_SHIFT 17 +#define PL353_SMC_SET_CYCLES_T6_MASK 0xF +#define PL353_SMC_SET_CYCLES_T6_SHIFT 20 + +/* ECC status register specific constants */ +#define PL353_SMC_ECC_STATUS_BUSY (1 << 6) + +/* ECC memory config register specific constants */ +#define PL353_SMC_ECC_MEMCFG_MODE_MASK 0xC +#define PL353_SMC_ECC_MEMCFG_MODE_SHIFT2 +#define PL353_SMC_ECC_MEMCFG_PGSIZE_MASK 0xC + +#define PL353_SMC_DC_UPT_NAND_REGS ((4 << 23) |/* CS: NAND chip */ \ +(2 << 21)) /* UpdateRegs operation */ + +#define PL353_NAND_ECC_CMD1((0x80) | /* Write command */ \ +(0 << 8) | /* Read
[PATCH RFC v3 2/2] memory: pl353: Add driver for arm pl353 static memory controller
Add driver for arm pl353 static memory controller. This controller is used in xilinx zynq soc for interfacing the nand and nor/sram memory devices. Signed-off-by: Punnaiah Choudary Kalluri punn...@xilinx.com --- Changes in v2: - Since now the timing parameters are in nano seconds, added logic to convert them to the cycles --- drivers/memory/Kconfig |6 + drivers/memory/Makefile |1 + drivers/memory/pl353-smc.c | 569 ++ include/linux/memory/pl353-smc.h | 34 +++ 4 files changed, 610 insertions(+), 0 deletions(-) create mode 100644 drivers/memory/pl353-smc.c create mode 100644 include/linux/memory/pl353-smc.h diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig index c59e9c9..58c29a2 100644 --- a/drivers/memory/Kconfig +++ b/drivers/memory/Kconfig @@ -65,4 +65,10 @@ config FSL_IFC bool depends on FSL_SOC +config PL353_SMC + bool ARM PL353 Static Memory Controller(SMC) driver + default y + depends on ARM + help + This driver is for the ARM PL353 Static Memory Controller(SMC) module. endif diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index 71160a2..5040977 100644 --- a/drivers/memory/Makefile +++ b/drivers/memory/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_IFC) += fsl_ifc.o obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o +obj-$(CONFIG_PL353_SMC)+= pl353-smc.o diff --git a/drivers/memory/pl353-smc.c b/drivers/memory/pl353-smc.c new file mode 100644 index 000..02221ba --- /dev/null +++ b/drivers/memory/pl353-smc.c @@ -0,0 +1,569 @@ +/* + * ARM PL353 SMC Driver + * + * Copyright (C) 2012 - 2014 Xilinx, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Currently only a single SMC instance is supported. + */ + +#include linux/clk.h +#include linux/io.h +#include linux/kernel.h +#include linux/memory/pl353-smc.h +#include linux/module.h +#include linux/of_platform.h +#include linux/platform_device.h +#include linux/slab.h + +/* Register definitions */ +#define PL353_SMC_MEMC_STATUS_OFFS 0 /* Controller status reg, RO */ +#define PL353_SMC_CFG_CLR_OFFS 0xC /* Clear config reg, WO */ +#define PL353_SMC_DIRECT_CMD_OFFS 0x10/* Direct command reg, WO */ +#define PL353_SMC_SET_CYCLES_OFFS 0x14/* Set cycles register, WO */ +#define PL353_SMC_SET_OPMODE_OFFS 0x18/* Set opmode register, WO */ +#define PL353_SMC_ECC_STATUS_OFFS 0x400 /* ECC status register */ +#define PL353_SMC_ECC_MEMCFG_OFFS 0x404 /* ECC mem config reg */ +#define PL353_SMC_ECC_MEMCMD1_OFFS 0x408 /* ECC mem cmd1 reg */ +#define PL353_SMC_ECC_MEMCMD2_OFFS 0x40C /* ECC mem cmd2 reg */ +#define PL353_SMC_ECC_VALUE0_OFFS 0x418 /* ECC value 0 reg */ + +/* Controller status register specifc constants */ +#define PL353_SMC_MEMC_STATUS_RAW_INT_1_SHIFT 6 + +/* Clear configuration register specific constants */ +#define PL353_SMC_CFG_CLR_INT_CLR_10x10 +#define PL353_SMC_CFG_CLR_ECC_INT_DIS_10x40 +#define PL353_SMC_CFG_CLR_INT_DIS_10x2 +#define PL353_SMC_CFG_CLR_DEFAULT_MASK (PL353_SMC_CFG_CLR_INT_CLR_1 | \ +PL353_SMC_CFG_CLR_ECC_INT_DIS_1 | \ +PL353_SMC_CFG_CLR_INT_DIS_1) + +/* Set cycles register specific constants */ +#define PL353_SMC_SET_CYCLES_T0_MASK 0xF +#define PL353_SMC_SET_CYCLES_T0_SHIFT 0 +#define PL353_SMC_SET_CYCLES_T1_MASK 0xF +#define PL353_SMC_SET_CYCLES_T1_SHIFT 4 +#define PL353_SMC_SET_CYCLES_T2_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T2_SHIFT 8 +#define PL353_SMC_SET_CYCLES_T3_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T3_SHIFT 11 +#define PL353_SMC_SET_CYCLES_T4_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T4_SHIFT 14 +#define PL353_SMC_SET_CYCLES_T5_MASK 0x7 +#define PL353_SMC_SET_CYCLES_T5_SHIFT 17 +#define PL353_SMC_SET_CYCLES_T6_MASK 0xF +#define PL353_SMC_SET_CYCLES_T6_SHIFT 20 + +/* ECC status register specific constants */ +#define PL353_SMC_ECC_STATUS_BUSY (1 6) + +/* ECC memory config register specific constants */ +#define PL353_SMC_ECC_MEMCFG_MODE_MASK 0xC +#define PL353_SMC_ECC_MEMCFG_MODE_SHIFT2 +#define PL353_SMC_ECC_MEMCFG_PGSIZE_MASK 0xC + +#define PL353_SMC_DC_UPT_NAND_REGS ((4 23) |/* CS: NAND chip */ \ +(2 21)) /* UpdateRegs
