Xilinx Zynq OCM support

PATCH In review Michal Simek 2 participants 10 messages Dec 1, 2014 – Dec 2, 2014

Subsystems (4)

Device tree bindings Arm (32-bit) Arm sub-architectures Arm/Zynq architecture

lore

Share & tools

lore

Michal Simek

author

2014-12-01 13:24 UTC #0 lore raw

v5 cover Xilinx Zynq OCM support

Hi,

this is the next attepmt to add support for On Chip Memory configuration via On Chip Memory Controller.
OCM can be divided into 4 independend blocks and placed to two locations which this driver detects.

For everybody on-chip SRAM driver "mmio-sram"
is missing parity IRQ handling not sure how to write
in generic way and also the memory layout
can be changed at run time (not currently supported by this driver)

smp-sram trampoline allocation can be used from mmio-sram and size allocated via DT but currently no reason for using it.

Creating mmio-sram node with setting at run time based on current setting is possible but it won't look good.
One way how to do it is here
"ARM: mvebu: Add quirk for i2c for the OpenBlocks AX3-4 board" (sha1: 85e618a1be2b2092318178d1d66bdad49cbbeeeb)
but creating nodes at run-time or changing compact strings will be just more hacky code.

Using device-tree overlays is another option but SMP trampoline code is placed there and it will be used by PM code that's why driver should be there before user-space.

Next option is to create driver which just create platform device at run-time. This is doable but I expect sram driver has to be updated.

Regarding reading SLCR OCM configuration there is an option to use regmap and not to use zynq_slcr_get_ocm_config.
This is how that code will look like when regmap is used.

+ struct regmap *syscon;
+
+ syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "regmap");
+ if (IS_ERR(syscon)) {
+ dev_err(&pdev->dev, "regmap property not found\n");
+ return PTR_ERR(syscon);
+ }

- ocm_config = zynq_slcr_get_ocm_config();
+ ret = regmap_read(syscon, SLCR_OCM_CFG_OFFSET,
+ &ocm_config);
+ if (ret)
+ return -ENODEV;

Not sure which option is better but this is also working fine. With this change patch
"ARM: zynq: Extend SLCR driver to read OCM configuration" is not needed.

Regarding ifdef CONFIG_SMP driver allocates just size which is used by the kernel - there could be an option to allocate fixed size but it is IMHO worse then allocate actual needed part.
With this simplification there is no need to refactor common.h but it doesn't mean that refactoring is not good to do.

Thanks,
Michal

Changes in v5:
- Separate binding from the driver
- Fix commit message reported by Linus W
- Use SZ_64K instead size in HEX reported by Linus W
- Fix dev_dbg message for allocate reset vector table
- Detect start address not base for SMP trampoline allocation
- Move binding to separate patch

Changes in v4:
- New patch in this series
- New patch in this series
- Move only slcr.h and smp.h and keep common.h in platform which is not needed by OCMC driver
Based on Arnd request: https://lkml.org/lkml/2014/10/20/268
Not all symbols from slcr.h/smp.h will be used by OCMC driver
- no problem to remove them if it is needed
- Add record to MAINTAINERS file
- slcr.h has moved to soc/include/ folder. Move definition there too.
- Use }; instead of } ; in doc
- Use memory-controller at ... instead of ocmc at ...
- Create Kconfig entry for OCMC driver - enable GENERIC_ALLOCATOR here
- Add entry to MAINTAINERS file
- Use memory-controller at ... instead of ocmc at ...

Changes in v3:
- Move OCM to drivers/soc
- Update year
- Extract DTS node to be able to apply it out of driver
- Remove generic allocator enabling
- Extract SLCR part
- Use const in of_device_id
- OCM->OCMC
- Use ocmc-1.0 compatible string
- Extract from OCM driver

Changes in v2:
- Update pm.c added in 3.17 too - Soren pointed on it
- Change compatibility string to be in xilinx format
- Fix kernel-doc format

Michal Simek (7):
ARM: zynq: Extract smp related functions out of common.h
ARM: zynq: Extract slcr related functions out of common.h
ARM: zynq: Move slcr.h and smp.h to generic location
ARM: zynq: Extend SLCR driver to read OCM configuration
devicetree: bindings: Add zynq ocmc node description
ARM: zynq: Add OCM controller driver
ARM: zynq: DT: Add OCM controller node

 .../bindings/arm/zynq/xlnx,zynq-ocmc.txt           |  17 ++
 MAINTAINERS                                        |   2 +
 arch/arm/boot/dts/zynq-7000.dtsi                   |   7 +
 arch/arm/mach-zynq/common.c                        |   2 +
 arch/arm/mach-zynq/common.h                        |  19 --
 arch/arm/mach-zynq/platsmp.c                       |   3 +
 arch/arm/mach-zynq/slcr.c                          |  17 ++
 drivers/soc/Kconfig                                |   1 +
 drivers/soc/Makefile                               |   1 +
 drivers/soc/zynq/Kconfig                           |  13 ++
 drivers/soc/zynq/Makefile                          |   1 +
 drivers/soc/zynq/zynq_ocmc.c                       | 249 +++++++++++++++++++++
 include/soc/zynq/slcr.h                            |  30 +++
 include/soc/zynq/smp.h                             |  30 +++
 14 files changed, 373 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt
 create mode 100644 drivers/soc/zynq/Kconfig
 create mode 100644 drivers/soc/zynq/Makefile
 create mode 100644 drivers/soc/zynq/zynq_ocmc.c
 create mode 100644 include/soc/zynq/slcr.h
 create mode 100644 include/soc/zynq/smp.h

--
1.8.2.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141201/7f131cf5/attachment.sig>

Michal Simek

author

2014-12-01 13:24 UTC #1 lore raw

PATCH v5 1/7 ARM: zynq: Extract smp related functions out of common.h

Create topic headers not to share all functions.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5: None
Changes in v4:
- New patch in this series

Changes in v3: None
Changes in v2: None

 arch/arm/mach-zynq/common.c  |  1 +
 arch/arm/mach-zynq/common.h  | 10 ----------
 arch/arm/mach-zynq/platsmp.c |  1 +
 arch/arm/mach-zynq/smp.h     | 30 ++++++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/mach-zynq/smp.h

arch/arm/mach-zynq/common.c +1 -0

index 26f92c28d22b..45cb778b344d 100644
@@ -43,6 +43,7 @@
 #include <asm/hardware/cache-l2x0.h>
 
 #include "common.h"
+#include "smp.h"
 
 #define ZYNQ_DEVCFG_MCTRL		0x80
 #define ZYNQ_DEVCFG_PS_VERSION_SHIFT	28

arch/arm/mach-zynq/common.h +0 ~~-10~~

index 382c60e9aa16..0667cc79f179 100644
@@ -17,8 +17,6 @@
 #ifndef __MACH_ZYNQ_COMMON_H__
 #define __MACH_ZYNQ_COMMON_H__
 
-void zynq_secondary_startup(void);
-
 extern int zynq_slcr_init(void);
 extern int zynq_early_slcr_init(void);
 extern void zynq_slcr_system_reset(void);

@@ -28,14 +26,6 @@ extern bool zynq_slcr_cpu_state_read(int cpu);
 extern void zynq_slcr_cpu_state_write(int cpu, bool die);
 extern u32 zynq_slcr_get_device_id(void);
 
-#ifdef CONFIG_SMP
-extern char zynq_secondary_trampoline;
-extern char zynq_secondary_trampoline_jump;
-extern char zynq_secondary_trampoline_end;
-extern int zynq_cpun_start(u32 address, int cpu);
-extern struct smp_operations zynq_smp_ops __initdata;
-#endif
-
 extern void __iomem *zynq_scu_base;
 
 void zynq_pm_late_init(void);

arch/arm/mach-zynq/platsmp.c +1 -0

index 52d768ff7857..5aa6b9ee95fc 100644
@@ -26,6 +26,7 @@
 #include <asm/smp_scu.h>
 #include <linux/irqchip/arm-gic.h>
 #include "common.h"
+#include "smp.h"
 
 /*
  * Store number of cores in the system

arch/arm/mach-zynq/smp.h new file +30 -0

new file mode 100644
index 000000000000..0277540e352c
@@ -0,0 +1,30 @@
+/*
+ * This file contains common function prototypes to avoid externs
+ * in the c files.
+ *
+ * Copyright (C) 2014 Xilinx
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#ifndef __MACH_ZYNQ_SMP_H__
+#define __MACH_ZYNQ_SMP_H__
+
+void zynq_secondary_startup(void);
+
+#ifdef CONFIG_SMP
+extern char zynq_secondary_trampoline;
+extern char zynq_secondary_trampoline_jump;
+extern char zynq_secondary_trampoline_end;
+extern int zynq_cpun_start(u32 address, int cpu);
+extern struct smp_operations zynq_smp_ops __initdata;
+#endif
+
+#endif

--
1.8.2.3

Michal Simek

author

2014-12-01 13:24 UTC #2 lore raw

PATCH v5 2/7 ARM: zynq: Extract slcr related functions out of common.h

In reply to Michal Simek

Create topic headers not to share all functions.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5: None
Changes in v4:
- New patch in this series

Changes in v3: None
Changes in v2: None

 arch/arm/mach-zynq/common.c  |  1 +
 arch/arm/mach-zynq/common.h  |  9 ---------
 arch/arm/mach-zynq/platsmp.c |  1 +
 arch/arm/mach-zynq/slcr.c    |  1 +
 arch/arm/mach-zynq/slcr.h    | 29 +++++++++++++++++++++++++++++
 5 files changed, 32 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-zynq/slcr.h

arch/arm/mach-zynq/common.c +1 -0

index 45cb778b344d..947e72b79496 100644
@@ -43,6 +43,7 @@
 #include <asm/hardware/cache-l2x0.h>
 
 #include "common.h"
+#include "slcr.h"
 #include "smp.h"
 
 #define ZYNQ_DEVCFG_MCTRL		0x80

arch/arm/mach-zynq/common.h +0 -9

index 0667cc79f179..5557de83058a 100644
@@ -17,15 +17,6 @@
 #ifndef __MACH_ZYNQ_COMMON_H__
 #define __MACH_ZYNQ_COMMON_H__
 
-extern int zynq_slcr_init(void);
-extern int zynq_early_slcr_init(void);
-extern void zynq_slcr_system_reset(void);
-extern void zynq_slcr_cpu_stop(int cpu);
-extern void zynq_slcr_cpu_start(int cpu);
-extern bool zynq_slcr_cpu_state_read(int cpu);
-extern void zynq_slcr_cpu_state_write(int cpu, bool die);
-extern u32 zynq_slcr_get_device_id(void);
-
 extern void __iomem *zynq_scu_base;
 
 void zynq_pm_late_init(void);

arch/arm/mach-zynq/platsmp.c +1 -0

index 5aa6b9ee95fc..717ba2b922b1 100644
@@ -26,6 +26,7 @@
 #include <asm/smp_scu.h>
 #include <linux/irqchip/arm-gic.h>
 #include "common.h"
+#include "slcr.h"
 #include "smp.h"
 
 /*

arch/arm/mach-zynq/slcr.c +1 -0

index d4cb50cf97c0..40f8d7420334 100644
@@ -20,6 +20,7 @@
 #include <linux/regmap.h>
 #include <linux/clk/zynq.h>
 #include "common.h"
+#include "slcr.h"
 
 /* register offsets */
 #define SLCR_UNLOCK_OFFSET		0x8   /* SCLR unlock register */

arch/arm/mach-zynq/slcr.h new file +29 -0

new file mode 100644
index 000000000000..7b4edab666ee
@@ -0,0 +1,29 @@
+/*
+ * This file contains common function prototypes to avoid externs
+ * in the c files.
+ *
+ * Copyright (C) 2014 Xilinx
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#ifndef __MACH_ZYNQ_SLCR_H__
+#define __MACH_ZYNQ_SLCR_H__
+
+extern int zynq_slcr_init(void);
+extern int zynq_early_slcr_init(void);
+extern void zynq_slcr_system_reset(void);
+extern void zynq_slcr_cpu_stop(int cpu);
+extern void zynq_slcr_cpu_start(int cpu);
+extern bool zynq_slcr_cpu_state_read(int cpu);
+extern void zynq_slcr_cpu_state_write(int cpu, bool die);
+extern u32 zynq_slcr_get_device_id(void);
+
+#endif

--
1.8.2.3

Michal Simek

author

2014-12-01 13:24 UTC #3 lore raw

PATCH v5 3/7 ARM: zynq: Move slcr.h and smp.h to generic location

In reply to Michal Simek

include/soc/zynq/ is the right location for this header to be able to share information from this header with other drivers which require it.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5: None
Changes in v4:
- Move only slcr.h and smp.h and keep common.h in platform which is not needed by OCMC driver
Based on Arnd request: https://lkml.org/lkml/2014/10/20/268
Not all symbols from slcr.h/smp.h will be used by OCMC driver
- no problem to remove them if it is needed
- Add record to MAINTAINERS file

Changes in v3: None
Changes in v2:
- Update pm.c added in 3.17 too - Soren pointed on it

 MAINTAINERS                                     | 1 +
 arch/arm/mach-zynq/common.c                     | 4 ++--
 arch/arm/mach-zynq/platsmp.c                    | 5 +++--
 arch/arm/mach-zynq/slcr.c                       | 3 ++-
 {arch/arm/mach-zynq => include/soc/zynq}/slcr.h | 0
 {arch/arm/mach-zynq => include/soc/zynq}/smp.h  | 0
 6 files changed, 8 insertions(+), 5 deletions(-)
 rename {arch/arm/mach-zynq => include/soc/zynq}/slcr.h (100%)
 rename {arch/arm/mach-zynq => include/soc/zynq}/smp.h (100%)

MAINTAINERS +1 -0

index a20df9bf8ab0..7b6ed1f667ad 100644
@@ -1550,6 +1550,7 @@ S:	Supported
 F:	arch/arm/mach-zynq/
 F:	drivers/cpuidle/cpuidle-zynq.c
 F:	drivers/block/xsysace.c
+F:	include/soc/zynq/
 N:	zynq
 N:	xilinx
 F:	drivers/clocksource/cadence_ttc_timer.c

arch/arm/mach-zynq/common.c +2 -2

index 947e72b79496..d30ecc1a7641 100644
@@ -43,8 +43,8 @@
 #include <asm/hardware/cache-l2x0.h>
 
 #include "common.h"
-#include "slcr.h"
-#include "smp.h"
+#include <soc/zynq/slcr.h>
+#include <soc/zynq/smp.h>
 
 #define ZYNQ_DEVCFG_MCTRL		0x80
 #define ZYNQ_DEVCFG_PS_VERSION_SHIFT	28

arch/arm/mach-zynq/platsmp.c +3 -2

index 717ba2b922b1..6d2b8a16b6c3 100644
@@ -25,9 +25,10 @@
 #include <asm/cacheflush.h>
 #include <asm/smp_scu.h>
 #include <linux/irqchip/arm-gic.h>
+
 #include "common.h"
-#include "slcr.h"
-#include "smp.h"
+#include <soc/zynq/slcr.h>
+#include <soc/zynq/smp.h>
 
 /*
  * Store number of cores in the system

arch/arm/mach-zynq/slcr.c +2 -1

index 40f8d7420334..b77e42d999c8 100644
@@ -20,7 +20,8 @@
 #include <linux/regmap.h>
 #include <linux/clk/zynq.h>
 #include "common.h"
-#include "slcr.h"
+#include <soc/zynq/slcr.h>
+#include <soc/zynq/smp.h>
 
 /* register offsets */
 #define SLCR_UNLOCK_OFFSET		0x8   /* SCLR unlock register */

include/soc/zynq/slcr.h renamed from arch/arm/mach-zynq/slcr.h +0 -0

similarity index 100%
rename from arch/arm/mach-zynq/slcr.h
rename to include/soc/zynq/slcr.h

include/soc/zynq/smp.h renamed from arch/arm/mach-zynq/smp.h +0 -0

similarity index 100%
rename from arch/arm/mach-zynq/smp.h
rename to include/soc/zynq/smp.h

--
1.8.2.3

Michal Simek

author

2014-12-01 13:24 UTC #4 lore raw

PATCH v5 4/7 ARM: zynq: Extend SLCR driver to read OCM configuration

In reply to Michal Simek

Get OCM configuration from SLCR.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5: None
Changes in v4:
- slcr.h has moved to soc/include/ folder. Move definition there too.

Changes in v3: None
Changes in v2: None

 arch/arm/mach-zynq/slcr.c | 15 +++++++++++++++
 include/soc/zynq/slcr.h   |  1 +
 2 files changed, 16 insertions(+)

arch/arm/mach-zynq/slcr.c +15 -0

index b77e42d999c8..f7c606e57525 100644
@@ -29,6 +29,7 @@
 #define SLCR_A9_CPU_RST_CTRL_OFFSET	0x244 /* CPU Software Reset Control */
 #define SLCR_REBOOT_STATUS_OFFSET	0x258 /* PS Reboot Status */
 #define SLCR_PSS_IDCODE			0x530 /* PS IDCODE */
+#define SLCR_OCM_CFG_OFFSET		0x910 /* OCM Address Mapping */
 
 #define SLCR_UNLOCK_MAGIC		0xDF0D
 #define SLCR_A9_CPU_CLKSTOP		0x10

@@ -128,6 +129,20 @@ void zynq_slcr_system_reset(void)
 }
 
 /**
+ * zynq_slcr_get_ocm_config - Get SLCR OCM config
+ *
+ * Return:	OCM config bits
+ */
+u32 zynq_slcr_get_ocm_config(void)
+{
+	u32 val;
+
+	zynq_slcr_read(&val, SLCR_OCM_CFG_OFFSET);
+
+	return val;
+}
+
+/**
  * zynq_slcr_cpu_start - Start cpu
  * @cpu:	cpu number
  */

include/soc/zynq/slcr.h +1 -0

index 7b4edab666ee..639723c4f90c 100644
@@ -25,5 +25,6 @@ extern void zynq_slcr_cpu_start(int cpu);
 extern bool zynq_slcr_cpu_state_read(int cpu);
 extern void zynq_slcr_cpu_state_write(int cpu, bool die);
 extern u32 zynq_slcr_get_device_id(void);
+extern u32 zynq_slcr_get_ocm_config(void);
 
 #endif

--
1.8.2.3

Michal Simek

author

2014-12-01 13:24 UTC #5 lore raw

PATCH v5 5/7 devicetree: bindings: Add zynq ocmc node description

In reply to Michal Simek

Add binding for Zynq On Chip Memory controller driver which is taking care about On Chip Memory.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5:
- Separate binding from the driver

Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt

Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt new file +17 -0

new file mode 100644
index 000000000000..dcf221e8a16e
@@ -0,0 +1,17 @@
+Device tree bindings for Zynq's OCM controller
+
+The OCM is divided to 4 64kB segments which can be separately configured
+to low or high location. Location is controlled via SLCR.
+
+Required properties:
+ compatible: Compatibility string. Must be "xlnx,zynq-ocmc-1.0".
+ reg: Specify the base and size of the OCMC registers in the memory map.
+      E.g.: reg = <0xf800c000 0x1000>;
+
+Example:
+ocmc: memory-controller at f800c000 {
+	compatible =  "xlnx,zynq-ocmc-1.0";
+	interrupt-parent = <&intc>;
+	interrupts = <0 3 4>;
+	reg = <0xf800c000 0x1000>;
+};

--
1.8.2.3

Michal Simek

author

2014-12-01 13:24 UTC #6 lore raw

PATCH v5 6/7 ARM: zynq: Add OCM controller driver

In reply to Michal Simek

The driver provide memory allocator which can be used by others drivers to allocate memory inside OCM. All locations for 64kB blocks are supported
and driver is trying to allocate the largest continuous block of memory.

Checking mpcore addressing filterring is not done here but could be added in future.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5:
- Fix commit message reported by Linus W
- Use SZ_64K instead size in HEX reported by Linus W
- Fix dev_dbg message for allocate reset vector table
- Detect start address not base for SMP trampoline allocation
- Move binding to separate patch

Changes in v4:
- Use }; instead of } ; in doc
- Use memory-controller at ... instead of ocmc at ...
- Create Kconfig entry for OCMC driver - enable GENERIC_ALLOCATOR here
- Add entry to MAINTAINERS file

Changes in v2:
- Change compatibility string to be in xilinx format
- Fix kernel-doc format

 MAINTAINERS                  |   1 +
 drivers/soc/Kconfig          |   1 +
 drivers/soc/Makefile         |   1 +
 drivers/soc/zynq/Kconfig     |  13 +++
 drivers/soc/zynq/Makefile    |   1 +
 drivers/soc/zynq/zynq_ocmc.c | 249 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 266 insertions(+)
 create mode 100644 drivers/soc/zynq/Kconfig
 create mode 100644 drivers/soc/zynq/Makefile
 create mode 100644 drivers/soc/zynq/zynq_ocmc.c

MAINTAINERS +1 -0

index 7b6ed1f667ad..ad56d68273fe 100644
@@ -1550,6 +1550,7 @@ S:	Supported
 F:	arch/arm/mach-zynq/
 F:	drivers/cpuidle/cpuidle-zynq.c
 F:	drivers/block/xsysace.c
+F:	drivers/soc/zynq/
 F:	include/soc/zynq/
 N:	zynq
 N:	xilinx

drivers/soc/Kconfig +1 -0

index 76d6bd4da138..25cc114a982d 100644
@@ -3,5 +3,6 @@ menu "SOC (System On Chip) specific Drivers"
 source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/ti/Kconfig"
 source "drivers/soc/versatile/Kconfig"
+source "drivers/soc/zynq/Kconfig"
 
 endmenu

drivers/soc/Makefile +1 -0

index 063113d0bd38..77a7865a3367 100644
@@ -6,3 +6,4 @@ obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-$(CONFIG_SOC_TI)		+= ti/
 obj-$(CONFIG_PLAT_VERSATILE)	+= versatile/
+obj-$(CONFIG_ARCH_ZYNQ)		+= zynq/

drivers/soc/zynq/Kconfig new file +13 -0

new file mode 100644
index 000000000000..0163c54acbc6
@@ -0,0 +1,13 @@
+#
+# Xilnx Zynq SoC drivers
+#
+
+if ARCH_ZYNQ
+
+config ZYNQ_OCMC
+	def_bool y
+	select GENERIC_ALLOCATOR
+	help
+	  Xilinx Zynq On Chip Memory Controller driver
+
+endif

drivers/soc/zynq/Makefile new file +1 -0

new file mode 100644
index 000000000000..807c9b83850d
@@ -0,0 +1 @@
+obj-$(CONFIG_ZYNQ_OCMC) += zynq_ocmc.o

drivers/soc/zynq/zynq_ocmc.c new file +249 -0

249 lines not on this page · raw message

--
1.8.2.3

Arnd Bergmann

2014-12-01 15:31 UTC #7 lore raw

Re: [PATCH v5 6/7] ARM: zynq: Add OCM controller driver

Quoted 10 lines · On Monday 01 December 2014 14:24:57 Michal Simek wrote:

The driver provide memory allocator which can be used by others drivers to allocate memory inside OCM. All locations for 64kB blocks are supported
and driver is trying to allocate the largest continuous block of memory.
Checking mpcore addressing filterring is not done here but could be added in future.
Signed-off-by Michal Simek <[email protected]>

The driver doesn't actually have any interface as far as I can tell, so I wonder how you expect it to be used.

Do you have a list of drivers that would be using it?

How does it relate to the generic "mmio-sram" binding? Is this meant as a specialization?

Quoted 16 lines

drivers/soc/zynq/zynq_ocmc.c new file +16 -0

+/**
+ * zynq_ocmc_irq_handler - Interrupt service routine of the OCM controller
+ * @irq:	IRQ number
+ * @data:	Pointer to the zynq_ocmc_dev structure
+ *
+ * Return:     IRQ_HANDLED always
+ */
+static irqreturn_t zynq_ocmc_irq_handler(int irq, void *data)
+{
+	u32 sts;
+	u32 err_addr;
+	struct zynq_ocmc_dev *zynq_ocmc = data;
+
+	/* check status */
+	sts = readl(zynq_ocmc->base + ZYNQ_OCMC_IRQ_STS);
+	if (sts & ZYNQ_OCMC_IRQ_STS_ERR_MASK) {

drivers/soc/zynq/zynq_ocmc.c line 76

Quoted 10 lines

drivers/soc/zynq/zynq_ocmc.c new file +10 -0

+		/* check error address */
+		err_addr = readl(zynq_ocmc->base + ZYNQ_OCMC_PARITY_ERRADDRESS);
+		pr_err("%s: OCM err intr generated at 0x%04x (stat: 0x%08x).",
+		       __func__, err_addr, sts & ZYNQ_OCMC_IRQ_STS_ERR_MASK);
+	}
+	pr_warn("%s: Interrupt generated by OCM, but no error is found.",
+		__func__);
+
+	return IRQ_HANDLED;
+}

I'm also puzzled by this: you don't really do anything here but print a message. What is the purpose of this interrupt? As this looks unrelated to the rest of the driver, maybe this should be part of drivers/edac instead?

Quoted 19 lines

drivers/soc/zynq/zynq_ocmc.c new file +10 -0

+static int zynq_ocmc_probe(struct platform_device *pdev)
+{
+       int ret;
+       struct zynq_ocmc_dev *zynq_ocmc;
+       u32 i, ocm_config, curr;
+       struct resource *res;
+
+       ocm_config = zynq_slcr_get_ocm_config();
+
+       zynq_ocmc = devm_kzalloc(&pdev->dev, sizeof(*zynq_ocmc),

GFP_KERNEL);

drivers/soc/zynq/zynq_ocmc.c new file +8 -0

+       if (!zynq_ocmc)
+               return -ENOMEM;
+
+       zynq_ocmc->pool = devm_gen_pool_create(&pdev->dev,
+                                              ilog2(ZYNQ_OCMC_GRANULARITY),
+                                              -1);
+       if (!zynq_ocmc->pool)
+               return -ENOMEM;

drivers/soc/zynq/zynq_ocmc.c line 113

Quoted 10 lines

drivers/soc/zynq/zynq_ocmc.c new file +10 -0

+
+       curr = 0; /* For storing current struct resource for OCM */
+       for (i = 0; i < ZYNQ_OCMC_BLOCKS; i++) {
+               u32 base, start, end;
+
+               /* Setup base address for 64kB OCM block */
+               if (ocm_config & BIT(i))
+                       base = ZYNQ_OCMC_HIGHADDR;
+               else
+                       base = ZYNQ_OCMC_LOWADDR;

You write in the introductory mail that you want to support detecting the address from the slcr, and possibly even allow changing it at runtime, but I don't understand what that would be good for.

It's not uncommon to describe in DT settings that one can also find out from hardware but that are set up by the boot loader in a particular way. Why not this one? For all I can tell, that would let you simply use one driver for the EDAC and the generic mmio-sram from the memory, without the need to do anything further.

Arnd

Michal Simek

author

2014-12-02 10:52 UTC #8 lore raw

Re: [PATCH v5 6/7] ARM: zynq: Add OCM controller driver

Quoted 10 lines · Arnd Bergmann

Quoted 9 lines · On Monday 01 December 2014 14:24:57 Michal Simek wrote:
The driver provide memory allocator which can be used by others drivers to allocate memory inside OCM. All locations for 64kB blocks are supported
and driver is trying to allocate the largest continuous block of memory.
Checking mpcore addressing filterring is not done here but could be added in future.

Quoted 6 lines

Quoted 1 line
Signed-off-by Michal Simek <[email protected]>
The driver doesn't actually have any interface as far as I can tell, so I wonder how you expect it to be used.
Do you have a list of drivers that would be using it?

For supporting suspend you have to save content to OCM memory and you need any driver which is providing generic functionality to allocate that memory for it.

Here is example - it is not upstream because we need to get OCM driver to mainline first.
https://github.com/Xilinx/linux-xlnx/blob/master/arch/arm/mach-zynq/pm.c

The second example is to use OCM memory for storing BD for ethernet controller. OCM is fast memory accessible IRC in one cycle which increase bandwidth.

For all of these you need the driver which provide you the way how to allocate memory from OCM.

Quoted 2 lines

How does it relate to the generic "mmio-sram" binding? Is this meant as a specialization?

This driver was based on mmio-sram which does the same. Provide you a way how to allocate memory space in any sram memory on chip.
It is design for static case but not for dynamic one like this one.

As I pointed in cover letter there could be an option to use mmio-sram driver but there must be any way how to create DT description at run-time or choose configuration at run-time.

Quoted 23 lines

drivers/soc/zynq/zynq_ocmc.c new file +23 -0

+/**
+ * zynq_ocmc_irq_handler - Interrupt service routine of the OCM controller
+ * @irq:	IRQ number
+ * @data:	Pointer to the zynq_ocmc_dev structure
+ *
+ * Return:     IRQ_HANDLED always
+ */
+static irqreturn_t zynq_ocmc_irq_handler(int irq, void *data)
+{
+	u32 sts;
+	u32 err_addr;
+	struct zynq_ocmc_dev *zynq_ocmc = data;
+
+	/* check status */
+	sts = readl(zynq_ocmc->base + ZYNQ_OCMC_IRQ_STS);
+	if (sts & ZYNQ_OCMC_IRQ_STS_ERR_MASK) {
+		/* check error address */
+		err_addr = readl(zynq_ocmc->base + ZYNQ_OCMC_PARITY_ERRADDRESS);
+		pr_err("%s: OCM err intr generated at 0x%04x (stat: 0x%08x).",
+		       __func__, err_addr, sts & ZYNQ_OCMC_IRQ_STS_ERR_MASK);
+	}
+	pr_warn("%s: Interrupt generated by OCM, but no error is found.",
+		__func__);

Quoted 8 lines

drivers/soc/zynq/zynq_ocmc.c line 76
Quoted 3 lines
drivers/soc/zynq/zynq_ocmc.c new file +3 -0
+
+	return IRQ_HANDLED;
+}
I'm also puzzled by this: you don't really do anything here but print a message. What is the purpose of this interrupt? As this looks unrelated to the rest of the driver, maybe this should be part of drivers/edac instead?

yes - it is just showing that there is error in OCM.
It can be added to edac - no problem to do so but the driver will be really simple that's why not sure if make sense to do it.

Quoted 26 lines

drivers/soc/zynq/zynq_ocmc.c new file +10 -0

+static int zynq_ocmc_probe(struct platform_device *pdev)
+{
+       int ret;
+       struct zynq_ocmc_dev *zynq_ocmc;
+       u32 i, ocm_config, curr;
+       struct resource *res;
+
+       ocm_config = zynq_slcr_get_ocm_config();
+
+       zynq_ocmc = devm_kzalloc(&pdev->dev, sizeof(*zynq_ocmc),

GFP_KERNEL);

drivers/soc/zynq/zynq_ocmc.c new file +14 -0

+       if (!zynq_ocmc)
+               return -ENOMEM;
+
+       zynq_ocmc->pool = devm_gen_pool_create(&pdev->dev,
+                                              ilog2(ZYNQ_OCMC_GRANULARITY),
+                                              -1);
+       if (!zynq_ocmc->pool)
+               return -ENOMEM;
+
+       curr = 0; /* For storing current struct resource for OCM */
+       for (i = 0; i < ZYNQ_OCMC_BLOCKS; i++) {
+               u32 base, start, end;
+
+               /* Setup base address for 64kB OCM block */

Quoted 8 lines

drivers/soc/zynq/zynq_ocmc.c line 113
Quoted 4 lines
drivers/soc/zynq/zynq_ocmc.c new file +4 -0
+               if (ocm_config & BIT(i))
+                       base = ZYNQ_OCMC_HIGHADDR;
+               else
+                       base = ZYNQ_OCMC_LOWADDR;
You write in the introductory mail that you want to support detecting the address from the slcr, and possibly even allow changing it at runtime, but I don't understand what that would be good for.

Note: PL means programmable logic

On zynq you can use memory from fixed part which starts from 0 to ddr size. That's one configuration.
The second is to add memory controller just to PL - the use case can be that memory controller in PL will provide better bandwidth (because of customization) for your application and this memory can't started from 0. Most of the time it starts from 0x40000000. But you need to have memory at 0 for adding reset vectors or standalone application running on the second ARM core that's why you can place there OCM.

It means it is up to users what configuration they want/need to use that's why OCM is divided to 4 64k blocks which you can place to any location you like. it is 16 combinations.

Chip can be setup to use any of this combination without any capable bootloader that's why relating on bootloader is not solution. Also problematic for upgrading the system.

Quoted 3 lines

It's not uncommon to describe in DT settings that one can also find out from hardware but that are set up by the boot loader in a particular way. Why not this one?

It is 16 combinations how these 4 blocks can be placed together but only one reliable way is to check SLCR configuration and based on reg setup.

I can add all these 16 combinations to DTS and detect configuration in SLCR and then change any status property for that particular configuration but it will be more ugly than this solution.

Note: Just place there 4/8 blocks with low and high locations is not good because the maximum allocated size will be just 64k.

Quoted 3 lines

For all I can tell, that would let you simply
use one driver for the EDAC and the generic mmio-sram from the memory, without the need to do anything further.

EDAC driver - no problem to do it and make sense.
Using mmio-sram for one fixed configuration is easy but not sure if this is the right thing to do.
Listing all combinations in DTS file is quite easy to do but mechanism to select one combination is the key for it.

Thanks,
Michal

Michal Simek

author

2014-12-01 13:24 UTC #9 lore raw

PATCH v5 7/7 ARM: zynq: DT: Add OCM controller node

In reply to Michal Simek

Add the on-chip-memory controller node to the Zynq devicetree.

Signed-off-by Michal Simek <[email protected]>

---

Changes in v5: None
Changes in v4:
- Use memory-controller at ... instead of ocmc at ...

Changes in v3:
- Extract from OCM driver

Changes in v2: None

 arch/arm/boot/dts/zynq-7000.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

arch/arm/boot/dts/zynq-7000.dtsi +7 -0

index 24036c440440..040f95e817c8 100644
@@ -151,6 +151,13 @@
 			reg = <0xf8006000 0x1000>;
 		} ;
 
+		ocmc: memory-controller at f800c000 {
+			compatible = "xlnx,zynq-ocmc-1.0";
+			interrupt-parent = <&intc>;
+			interrupts = <0 3 4>;
+			reg = <0xf800c000 0x1000>;
+		};
+
 		uart0: serial at e0000000 {
 			compatible = "xlnx,xuartps", "cdns,uart-r1p8";
 			status = "disabled";

--
1.8.2.3