2017-12-14 05:56:08

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v3 0/2] Documentation and driver of logicoreIP

1st patch provide Device Tree binding document for logicoreIP
2nd patch provide the xlnx_vcu logicoreIP driver, Kconfig changes
and Makefile changes for the driver.

Dhaval Shah (2):
Documentation: devicetree: Add DT bindings to xlnx_vcu driver
misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 +
drivers/misc/Kconfig | 15 +
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 629 +++++++++++++++++++++
4 files changed, 676 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt
create mode 100644 drivers/misc/xlnx_vcu.c

--
2.7.4


2017-12-14 05:56:18

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v3 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

Add Device Tree binding document for logicoreIP. This logicoreIP
provides the isolation between the processing system and
programmable logic. Also provides the clock related information.

Signed-off-by: Dhaval Shah <[email protected]>
---
Chnages since v3:
* Use "dt-bindings: misc: ..." for the subject.
Changes since v2:
* Describe the h/w
* compatible string is updated to make it more specific
based on the logicoreIP version.
* Removed that encoder and decoder child nodes and relatd properties as that
will be a separate driver and dts nodes. other team is working on that.
* Updated to use as a single driver.

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt

diff --git a/Documentation/devicetree/bindings/misc/xlnx,vcu.txt b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
new file mode 100644
index 0000000..6786d67
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
@@ -0,0 +1,31 @@
+LogicoreIP designed compatible with Xilinx ZYNQ family.
+-------------------------------------------------------
+
+General concept
+---------------
+
+LogicoreIP design to provide the isolation between processing system
+and programmable logic. Also provides the list of register set to configure
+the frequency.
+
+Required properties:
+- compatible: shall be one of:
+ "xlnx,vcu"
+ "xlnx,vcu-logicoreip-1.0"
+- reg, reg-names: There are two sets of registers need to provide.
+ 1. vcu slcr
+ 2. Logicore
+ reg-names should contain name for the each register sequence.
+- clocks: phandle for aclk and pll_ref clocksource
+- clock-names: The identification string, "aclk", is always required for
+ the axi clock. "pll_ref" is required for pll.
+Example:
+
+ xlnx_vcu: vcu@a0040000 {
+ compatible = "xlnx,vcu-logicoreip-1.0";
+ reg = <0x0 0xa0040000 0x0 0x1000>,
+ <0x0 0xa0041000 0x0 0x1000>;
+ reg-names = "vcu_slcr", "logicore";
+ clocks = <&si570_1>, <&clkc 71>;
+ clock-names = "pll_ref", "aclk";
+ };
--
2.7.4

2017-12-14 05:56:49

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v3 2/2] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Xilinx ZYNQMP logicoreIP Init driver is based on the new
LogiCoreIP design created. This driver provides the processing system
and programmable logic isolation. Set the frequency based on the clock
information get from the logicoreIP register set.

It is put in drivers/misc as there is no subsystem for this logicoreIP.

Signed-off-by: Dhaval Shah <[email protected]>
---
Changes since v3:
No Changes.
Changes since v2:
* Removed the "default n" from the Kconfig
* More help text added to explain more about the logicoreIP driver
* SPDX id is relocated at top of the file with // style comment
* Removed the export API and header file and make it a single driver
which provides logocoreIP init.
* Provide the information in commit message as well for the why driver
in drivers/misc.

drivers/misc/Kconfig | 15 ++
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 629 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 645 insertions(+)
create mode 100644 drivers/misc/xlnx_vcu.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f1a5c23..24ea516 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
Enable this configuration option to enable the host side test driver
for PCI Endpoint.

+config XILINX_VCU
+ tristate "Xilinx VCU logicoreIP Init"
+ help
+ Provides the driver to enable and disable the isolation between the
+ processing system and programmable logic part by using the logicoreIP
+ register set. This driver also configure the frequency based on the
+ clock information get from the logicoreIP register set.
+
+ If you say yes here you get support for the logcoreIP.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called xlnx_vcu.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 5ca5f64..a6bd0b1 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
+obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o

lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
new file mode 100644
index 0000000..41819f0
--- /dev/null
+++ b/drivers/misc/xlnx_vcu.c
@@ -0,0 +1,629 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+/* Address map for different registers implemented in the VCU LogiCORE IP. */
+#define VCU_ECODER_ENABLE 0x00
+#define VCU_DECODER_ENABLE 0x04
+#define VCU_MEMORY_DEPTH 0x08
+#define VCU_ENC_COLOR_DEPTH 0x0c
+#define VCU_ENC_VERTICAL_RANGE 0x10
+#define VCU_ENC_FRAME_SIZE_X 0x14
+#define VCU_ENC_FRAME_SIZE_Y 0x18
+#define VCU_ENC_COLOR_FORMAT 0x1c
+#define VCU_ENC_FPS 0x20
+#define VCU_MCU_CLK 0x24
+#define VCU_CORE_CLK 0x28
+#define VCU_PLL_BYPASS 0x2c
+#define VCU_ENC_CLK 0x30
+#define VCU_PLL_CLK 0x34
+#define VCU_ENC_VIDEO_STANDARD 0x38
+#define VCU_STATUS 0x3c
+#define VCU_AXI_ENC_CLK 0x40
+#define VCU_AXI_DEC_CLK 0x44
+#define VCU_AXI_MCU_CLK 0x48
+#define VCU_DEC_VIDEO_STANDARD 0x4c
+#define VCU_DEC_FRAME_SIZE_X 0x50
+#define VCU_DEC_FRAME_SIZE_Y 0x54
+#define VCU_DEC_FPS 0x58
+#define VCU_BUFFER_B_FRAME 0x5c
+#define VCU_WPP_EN 0x60
+#define VCU_PLL_CLK_DEC 0x64
+#define VCU_GASKET_INIT 0x74
+#define VCU_GASKET_VALUE 0x03
+
+/* vcu slcr registers, bitmask and shift */
+#define VCU_PLL_CTRL 0x24
+#define VCU_PLL_CTRL_RESET_MASK 0x01
+#define VCU_PLL_CTRL_RESET_SHIFT 0
+#define VCU_PLL_CTRL_BYPASS_MASK 0x01
+#define VCU_PLL_CTRL_BYPASS_SHIFT 3
+#define VCU_PLL_CTRL_FBDIV_MASK 0x7f
+#define VCU_PLL_CTRL_FBDIV_SHIFT 8
+#define VCU_PLL_CTRL_POR_IN_MASK 0x01
+#define VCU_PLL_CTRL_POR_IN_SHIFT 1
+#define VCU_PLL_CTRL_PWR_POR_MASK 0x01
+#define VCU_PLL_CTRL_PWR_POR_SHIFT 2
+#define VCU_PLL_CTRL_CLKOUTDIV_MASK 0x03
+#define VCU_PLL_CTRL_CLKOUTDIV_SHIFT 16
+#define VCU_PLL_CTRL_DEFAULT 0
+#define VCU_PLL_DIV2 2
+
+#define VCU_PLL_CFG 0x28
+#define VCU_PLL_CFG_RES_MASK 0x0f
+#define VCU_PLL_CFG_RES_SHIFT 0
+#define VCU_PLL_CFG_CP_MASK 0x0f
+#define VCU_PLL_CFG_CP_SHIFT 5
+#define VCU_PLL_CFG_LFHF_MASK 0x03
+#define VCU_PLL_CFG_LFHF_SHIFT 10
+#define VCU_PLL_CFG_LOCK_CNT_MASK 0x03ff
+#define VCU_PLL_CFG_LOCK_CNT_SHIFT 13
+#define VCU_PLL_CFG_LOCK_DLY_MASK 0x7f
+#define VCU_PLL_CFG_LOCK_DLY_SHIFT 25
+#define VCU_ENC_CORE_CTRL 0x30
+#define VCU_ENC_MCU_CTRL 0x34
+#define VCU_DEC_CORE_CTRL 0x38
+#define VCU_DEC_MCU_CTRL 0x3c
+#define VCU_PLL_DIVISOR_MASK 0x3f
+#define VCU_PLL_DIVISOR_SHIFT 4
+#define VCU_SRCSEL_MASK 0x01
+#define VCU_SRCSEL_SHIFT 0
+#define VCU_SRCSEL_PLL 1
+
+#define VCU_PLL_STATUS 0x60
+#define VCU_PLL_STATUS_LOCK_STATUS_MASK 0x01
+
+#define MHZ 1000000
+#define FVCO_MIN (1500U * MHZ)
+#define FVCO_MAX (3000U * MHZ)
+#define DIVISOR_MIN 0
+#define DIVISOR_MAX 63
+#define FRAC 100
+#define LIMIT (10 * MHZ)
+
+/**
+ * struct xvcu_device - Xilinx VCU init device structure
+ * @dev: Platform device
+ * @pll_ref: pll ref clock source
+ * @aclk: axi clock source
+ * @logicore_reg_ba: logicore reg base address
+ * @vcu_slcr_ba: vcu_slcr Register base address
+ * @coreclk: core clock frequency
+ */
+struct xvcu_device {
+ struct device *dev;
+ struct clk *pll_ref;
+ struct clk *aclk;
+ void __iomem *logicore_reg_ba;
+ void __iomem *vcu_slcr_ba;
+ u32 coreclk;
+};
+
+/**
+ * struct xvcu_pll_cfg - Helper data
+ * @fbdiv: The integer portion of the feedback divider to the PLL
+ * @cp: PLL charge pump control
+ * @res: PLL loop filter resistor control
+ * @lfhf: PLL loop filter high frequency capacitor control
+ * @lock_dly: Lock circuit configuration settings for lock windowsize
+ * @lock_cnt: Lock circuit counter setting
+ */
+struct xvcu_pll_cfg {
+ u32 fbdiv;
+ u32 cp;
+ u32 res;
+ u32 lfhf;
+ u32 lock_dly;
+ u32 lock_cnt;
+};
+
+static const struct xvcu_pll_cfg xvcu_pll_cfg[] = {
+ { 25, 3, 10, 3, 63, 1000 },
+ { 26, 3, 10, 3, 63, 1000 },
+ { 27, 4, 6, 3, 63, 1000 },
+ { 28, 4, 6, 3, 63, 1000 },
+ { 29, 4, 6, 3, 63, 1000 },
+ { 30, 4, 6, 3, 63, 1000 },
+ { 31, 6, 1, 3, 63, 1000 },
+ { 32, 6, 1, 3, 63, 1000 },
+ { 33, 4, 10, 3, 63, 1000 },
+ { 34, 5, 6, 3, 63, 1000 },
+ { 35, 5, 6, 3, 63, 1000 },
+ { 36, 5, 6, 3, 63, 1000 },
+ { 37, 5, 6, 3, 63, 1000 },
+ { 38, 5, 6, 3, 63, 975 },
+ { 39, 3, 12, 3, 63, 950 },
+ { 40, 3, 12, 3, 63, 925 },
+ { 41, 3, 12, 3, 63, 900 },
+ { 42, 3, 12, 3, 63, 875 },
+ { 43, 3, 12, 3, 63, 850 },
+ { 44, 3, 12, 3, 63, 850 },
+ { 45, 3, 12, 3, 63, 825 },
+ { 46, 3, 12, 3, 63, 800 },
+ { 47, 3, 12, 3, 63, 775 },
+ { 48, 3, 12, 3, 63, 775 },
+ { 49, 3, 12, 3, 63, 750 },
+ { 50, 3, 12, 3, 63, 750 },
+ { 51, 3, 2, 3, 63, 725 },
+ { 52, 3, 2, 3, 63, 700 },
+ { 53, 3, 2, 3, 63, 700 },
+ { 54, 3, 2, 3, 63, 675 },
+ { 55, 3, 2, 3, 63, 675 },
+ { 56, 3, 2, 3, 63, 650 },
+ { 57, 3, 2, 3, 63, 650 },
+ { 58, 3, 2, 3, 63, 625 },
+ { 59, 3, 2, 3, 63, 625 },
+ { 60, 3, 2, 3, 63, 625 },
+ { 61, 3, 2, 3, 63, 600 },
+ { 62, 3, 2, 3, 63, 600 },
+ { 63, 3, 2, 3, 63, 600 },
+ { 64, 3, 2, 3, 63, 600 },
+ { 65, 3, 2, 3, 63, 600 },
+ { 66, 3, 2, 3, 63, 600 },
+ { 67, 3, 2, 3, 63, 600 },
+ { 68, 3, 2, 3, 63, 600 },
+ { 69, 3, 2, 3, 63, 600 },
+ { 70, 3, 2, 3, 63, 600 },
+ { 71, 3, 2, 3, 63, 600 },
+ { 72, 3, 2, 3, 63, 600 },
+ { 73, 3, 2, 3, 63, 600 },
+ { 74, 3, 2, 3, 63, 600 },
+ { 75, 3, 2, 3, 63, 600 },
+ { 76, 3, 2, 3, 63, 600 },
+ { 77, 3, 2, 3, 63, 600 },
+ { 78, 3, 2, 3, 63, 600 },
+ { 79, 3, 2, 3, 63, 600 },
+ { 80, 3, 2, 3, 63, 600 },
+ { 81, 3, 2, 3, 63, 600 },
+ { 82, 3, 2, 3, 63, 600 },
+ { 83, 4, 2, 3, 63, 600 },
+ { 84, 4, 2, 3, 63, 600 },
+ { 85, 4, 2, 3, 63, 600 },
+ { 86, 4, 2, 3, 63, 600 },
+ { 87, 4, 2, 3, 63, 600 },
+ { 88, 4, 2, 3, 63, 600 },
+ { 89, 4, 2, 3, 63, 600 },
+ { 90, 4, 2, 3, 63, 600 },
+ { 91, 4, 2, 3, 63, 600 },
+ { 92, 4, 2, 3, 63, 600 },
+ { 93, 4, 2, 3, 63, 600 },
+ { 94, 4, 2, 3, 63, 600 },
+ { 95, 4, 2, 3, 63, 600 },
+ { 96, 4, 2, 3, 63, 600 },
+ { 97, 4, 2, 3, 63, 600 },
+ { 98, 4, 2, 3, 63, 600 },
+ { 99, 4, 2, 3, 63, 600 },
+ { 100, 4, 2, 3, 63, 600 },
+ { 101, 4, 2, 3, 63, 600 },
+ { 102, 4, 2, 3, 63, 600 },
+ { 103, 5, 2, 3, 63, 600 },
+ { 104, 5, 2, 3, 63, 600 },
+ { 105, 5, 2, 3, 63, 600 },
+ { 106, 5, 2, 3, 63, 600 },
+ { 107, 3, 4, 3, 63, 600 },
+ { 108, 3, 4, 3, 63, 600 },
+ { 109, 3, 4, 3, 63, 600 },
+ { 110, 3, 4, 3, 63, 600 },
+ { 111, 3, 4, 3, 63, 600 },
+ { 112, 3, 4, 3, 63, 600 },
+ { 113, 3, 4, 3, 63, 600 },
+ { 114, 3, 4, 3, 63, 600 },
+ { 115, 3, 4, 3, 63, 600 },
+ { 116, 3, 4, 3, 63, 600 },
+ { 117, 3, 4, 3, 63, 600 },
+ { 118, 3, 4, 3, 63, 600 },
+ { 119, 3, 4, 3, 63, 600 },
+ { 120, 3, 4, 3, 63, 600 },
+ { 121, 3, 4, 3, 63, 600 },
+ { 122, 3, 4, 3, 63, 600 },
+ { 123, 3, 4, 3, 63, 600 },
+ { 124, 3, 4, 3, 63, 600 },
+ { 125, 3, 4, 3, 63, 600 },
+};
+
+/**
+ * xvcu_read - Read from the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ *
+ * Return: Returns 32bit value from VCU register specified
+ *
+ */
+static u32 xvcu_read(void __iomem *iomem, u32 offset)
+{
+ return ioread32(iomem + offset);
+}
+
+/**
+ * xvcu_write - Write to the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @value: Value to write
+ */
+static void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
+{
+ iowrite32(value, iomem + offset);
+}
+
+/**
+ * xvcu_write_field_reg - Write to the vcu reg field
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @field: vcu reg field to write to
+ * @mask: vcu reg mask
+ * @shift: vcu reg number of bits to shift the bitfield
+ */
+static void xvcu_write_field_reg(void __iomem *iomem, int offset,
+ u32 field, u32 mask, int shift)
+{
+ u32 val = xvcu_read(iomem, offset);
+
+ val &= ~(mask << shift);
+ val |= (field & mask) << shift;
+
+ xvcu_write(iomem, offset, val);
+}
+
+/**
+ * xvcu_set_vcu_pll_info - Set the VCU PLL info
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Programming the VCU PLL based on the user configuration
+ * (ref clock freq, core clock freq, mcu clock freq).
+ * Core clock frequency has higher priority than mcu clock frequency
+ * Errors in following cases
+ * - When mcu or clock clock get from logicoreIP is 0
+ * - When VCU PLL DIV related bits value other than 1
+ * - When proper data not found for given data
+ * - When sis570_1 clocksource related operation failed
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
+{
+ u32 refclk, coreclk, mcuclk, inte, deci;
+ u32 divisor_mcu, divisor_core, fvco;
+ u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
+ u32 cfg_val, mod, ctrl;
+ int ret;
+ unsigned int i;
+ const struct xvcu_pll_cfg *found = NULL;
+
+ inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
+ deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
+ coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
+ mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
+ if (!mcuclk || !coreclk) {
+ dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
+ return -EINVAL;
+ }
+
+ refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
+ dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
+ dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ ret = clk_set_rate(xvcu->pll_ref, refclk);
+ if (ret)
+ dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
+ return ret;
+ }
+
+ refclk = clk_get_rate(xvcu->pll_ref);
+
+ /* The divide-by-2 should be always enabled (==1)
+ * to meet the timing in the design.
+ * Otherwise, it's an error
+ */
+ vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
+ clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
+ clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
+ if (clkoutdiv != 1) {
+ dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
+ return -EINVAL;
+ }
+
+ for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
+ const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
+
+ fvco = cfg->fbdiv * refclk;
+ if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
+ pll_clk = fvco / VCU_PLL_DIV2;
+ if (fvco % VCU_PLL_DIV2 != 0)
+ pll_clk++;
+ mod = pll_clk % coreclk;
+ if (mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ } else if (coreclk - mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ divisor_core++;
+ } else {
+ continue;
+ }
+ if (divisor_core >= DIVISOR_MIN &&
+ divisor_core <= DIVISOR_MAX) {
+ found = cfg;
+ divisor_mcu = pll_clk / mcuclk;
+ mod = pll_clk % mcuclk;
+ if (mcuclk - mod < LIMIT)
+ divisor_mcu++;
+ break;
+ }
+ }
+ }
+
+ if (!found) {
+ dev_err(xvcu->dev, "Invalid clock combination.\n");
+ return -EINVAL;
+ }
+
+ xvcu->coreclk = pll_clk / divisor_core;
+ mcuclk = pll_clk / divisor_mcu;
+ dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
+ dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
+
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
+ vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
+ VCU_PLL_CTRL_FBDIV_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
+ VCU_PLL_CTRL_POR_IN_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
+ VCU_PLL_CTRL_POR_IN_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+ /* Set divisor for the core and mcu clock */
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
+
+ /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
+ cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
+ (found->cp << VCU_PLL_CFG_CP_SHIFT) |
+ (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
+ (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
+ (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
+
+ return 0;
+}
+
+/**
+ * xvcu_set_pll - PLL init sequence
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Call the api to set the PLL info and once that is done then
+ * init the PLL sequence to make the PLL stable.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_pll(struct xvcu_device *xvcu)
+{
+ u32 lock_status;
+ unsigned long timeout;
+ int ret;
+
+ ret = xvcu_set_vcu_pll_info(xvcu);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to set pll info\n");
+ return ret;
+ }
+
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ /* Defined the timeout for the max time to wait the
+ * PLL_STATUS to be locked.
+ */
+ timeout = jiffies + msecs_to_jiffies(2000);
+ do {
+ lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
+ if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ return 0;
+ }
+ } while (!time_after(jiffies, timeout));
+
+ /* PLL is not locked even after the timeout of the 2sec */
+ dev_err(xvcu->dev, "PLL is not locked\n");
+ return -ETIMEDOUT;
+}
+
+/**
+ * xvcu_probe - Probe existence of the logicoreIP
+ * and initialize PLL
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct xvcu_device *xvcu;
+ int ret;
+
+ xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
+ if (!xvcu)
+ return -ENOMEM;
+
+ xvcu->dev = &pdev->dev;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
+ if (!res) {
+ dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->vcu_slcr_ba) {
+ dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
+ if (!res) {
+ dev_err(&pdev->dev, "get logicore memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->logicore_reg_ba) {
+ dev_err(&pdev->dev, "logicore register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
+ if (IS_ERR(xvcu->aclk)) {
+ dev_err(&pdev->dev, "Could not get aclk clock\n");
+ return PTR_ERR(xvcu->aclk);
+ }
+
+ xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
+ if (IS_ERR(xvcu->pll_ref)) {
+ dev_err(&pdev->dev, "Could not get pll_ref clock\n");
+ return PTR_ERR(xvcu->pll_ref);
+ }
+
+ ret = clk_prepare_enable(xvcu->aclk);
+ if (ret) {
+ dev_err(&pdev->dev, "aclk clock enable failed\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(&pdev->dev, "pll_ref clock enable failed\n");
+ goto error_aclk;
+ }
+
+ /* Do the Gasket isolation and put the VCU out of reset
+ * Bit 0 : Gasket isolation
+ * Bit 1 : put VCU out of reset
+ */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
+
+ /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
+ ret = xvcu_set_pll(xvcu);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to set the pll\n");
+ goto error_pll_ref;
+ }
+
+ dev_set_drvdata(&pdev->dev, xvcu);
+
+ dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
+
+ return 0;
+
+error_pll_ref:
+ clk_disable_unprepare(xvcu->pll_ref);
+error_aclk:
+ clk_disable_unprepare(xvcu->aclk);
+ return ret;
+}
+
+/**
+ * xvcu_remove - Insert gasket isolation
+ * and disable the clock
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_remove(struct platform_device *pdev)
+{
+ struct xvcu_device *xvcu;
+
+ xvcu = platform_get_drvdata(pdev);
+ if (!xvcu)
+ return -ENODEV;
+
+ /* Add the the Gasket isolation and put the VCU in reset.
+ */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ clk_disable_unprepare(xvcu->aclk);
+
+ return 0;
+}
+
+static const struct of_device_id xvcu_of_id_table[] = {
+ { .compatible = "xlnx,vcu" },
+ { .compatible = "xlnx,vcu-logicoreip-1.0" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
+
+static struct platform_driver xvcu_driver = {
+ .driver = {
+ .name = "xilinx-vcu",
+ .of_match_table = xvcu_of_id_table,
+ },
+ .probe = xvcu_probe,
+ .remove = xvcu_remove,
+};
+
+module_platform_driver(xvcu_driver);
+
+MODULE_AUTHOR("Dhaval Shah <[email protected]>");
+MODULE_DESCRIPTION("Xilinx VCU init Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2017-12-14 18:43:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On 12/13/2017 09:55 PM, Dhaval Shah wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
>
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
> Changes since v3:
> No Changes.
> Changes since v2:
> * Removed the "default n" from the Kconfig
> * More help text added to explain more about the logicoreIP driver
> * SPDX id is relocated at top of the file with // style comment
> * Removed the export API and header file and make it a single driver
> which provides logocoreIP init.
> * Provide the information in commit message as well for the why driver
> in drivers/misc.
>
> drivers/misc/Kconfig | 15 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/xlnx_vcu.c | 629 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 645 insertions(+)
> create mode 100644 drivers/misc/xlnx_vcu.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c23..24ea516 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test driver
> for PCI Endpoint.
>
> +config XILINX_VCU
> + tristate "Xilinx VCU logicoreIP Init"
> + help

Please indent the help text (below) by 2 additional spaces, as documented
in coding-style.rst.

> + Provides the driver to enable and disable the isolation between the
> + processing system and programmable logic part by using the logicoreIP
> + register set. This driver also configure the frequency based on the

configures

> + clock information get from the logicoreIP register set.

drop ^get^

> +
> + If you say yes here you get support for the logcoreIP.

logicoreIP.

> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called xlnx_vcu.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 5ca5f64..a6bd0b1 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> +obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o
>
> lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
> lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 0000000..41819f0
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,629 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts Dhaval Shah <[email protected]>
> + */
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +> +/* Address map for different registers implemented in the VCU LogiCORE IP. */

[snip]


> +/**
> + * xvcu_read - Read from the VCU register space
> + * @iomem: vcu reg space base address
> + * @offset: vcu reg offset from base
> + *
> + * Return: Returns 32bit value from VCU register specified
> + *
> + */
> +static u32 xvcu_read(void __iomem *iomem, u32 offset)

You could inline the read() and write() functions...

> +{
> + return ioread32(iomem + offset);
> +}
> +
> +/**
> + * xvcu_write - Write to the VCU register space
> + * @iomem: vcu reg space base address
> + * @offset: vcu reg offset from base
> + * @value: Value to write
> + */
> +static void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
> +{
> + iowrite32(value, iomem + offset);
> +}
> +
> +/**
> + * xvcu_write_field_reg - Write to the vcu reg field
> + * @iomem: vcu reg space base address
> + * @offset: vcu reg offset from base
> + * @field: vcu reg field to write to
> + * @mask: vcu reg mask
> + * @shift: vcu reg number of bits to shift the bitfield
> + */
> +static void xvcu_write_field_reg(void __iomem *iomem, int offset,
> + u32 field, u32 mask, int shift)
> +{
> + u32 val = xvcu_read(iomem, offset);
> +
> + val &= ~(mask << shift);
> + val |= (field & mask) << shift;
> +
> + xvcu_write(iomem, offset, val);
> +}
> +
> +/**
> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> + * @xvcu: Pointer to the xvcu_device structure
> + *
> + * Programming the VCU PLL based on the user configuration
> + * (ref clock freq, core clock freq, mcu clock freq).
> + * Core clock frequency has higher priority than mcu clock frequency
> + * Errors in following cases
> + * - When mcu or clock clock get from logicoreIP is 0
> + * - When VCU PLL DIV related bits value other than 1
> + * - When proper data not found for given data
> + * - When sis570_1 clocksource related operation failed
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
> +{
> + u32 refclk, coreclk, mcuclk, inte, deci;
> + u32 divisor_mcu, divisor_core, fvco;
> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> + u32 cfg_val, mod, ctrl;
> + int ret;
> + unsigned int i;
> + const struct xvcu_pll_cfg *found = NULL;
> +
> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> + if (!mcuclk || !coreclk) {
> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> + return -EINVAL;
> + }
> +
> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> +
> + clk_disable_unprepare(xvcu->pll_ref);
> + ret = clk_set_rate(xvcu->pll_ref, refclk);
> + if (ret)
> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> +
> + ret = clk_prepare_enable(xvcu->pll_ref);
> + if (ret) {
> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> + return ret;
> + }
> +
> + refclk = clk_get_rate(xvcu->pll_ref);
> +
> + /* The divide-by-2 should be always enabled (==1)

multi-line comment style:
/*
* The divide-by-2 should always be enabled (==1)

> + * to meet the timing in the design.
> + * Otherwise, it's an error
> + */
> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> + if (clkoutdiv != 1) {
> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> + return -EINVAL;
> + }
> +
> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
> + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> +
> + fvco = cfg->fbdiv * refclk;
> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> + pll_clk = fvco / VCU_PLL_DIV2;
> + if (fvco % VCU_PLL_DIV2 != 0)
> + pll_clk++;
> + mod = pll_clk % coreclk;
> + if (mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + } else if (coreclk - mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + divisor_core++;
> + } else {
> + continue;
> + }
> + if (divisor_core >= DIVISOR_MIN &&
> + divisor_core <= DIVISOR_MAX) {
> + found = cfg;
> + divisor_mcu = pll_clk / mcuclk;
> + mod = pll_clk % mcuclk;
> + if (mcuclk - mod < LIMIT)
> + divisor_mcu++;
> + break;
> + }
> + }
> + }
> +
> + if (!found) {
> + dev_err(xvcu->dev, "Invalid clock combination.\n");
> + return -EINVAL;
> + }
> +
> + xvcu->coreclk = pll_clk / divisor_core;
> + mcuclk = pll_clk / divisor_mcu;
> + dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> + dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> +
> + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
> + vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> + VCU_PLL_CTRL_FBDIV_SHIFT;
> + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
> + VCU_PLL_CTRL_POR_IN_SHIFT);
> + vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
> + VCU_PLL_CTRL_POR_IN_SHIFT;
> + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
> + VCU_PLL_CTRL_PWR_POR_SHIFT);
> + vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
> + VCU_PLL_CTRL_PWR_POR_SHIFT;
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
> +
> + /* Set divisor for the core and mcu clock */
> + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
> + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> + ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> + VCU_PLL_DIVISOR_SHIFT;
> + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
> +
> + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
> + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> + ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> + VCU_PLL_DIVISOR_SHIFT;
> + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
> +
> + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
> + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> + ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
> + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
> +
> + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
> + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> + ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
> + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
> +
> + /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
> + cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
> + (found->cp << VCU_PLL_CFG_CP_SHIFT) |
> + (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
> + (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
> + (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
> + xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
> +
> + return 0;
> +}
> +
> +/**
> + * xvcu_set_pll - PLL init sequence
> + * @xvcu: Pointer to the xvcu_device structure
> + *
> + * Call the api to set the PLL info and once that is done then
> + * init the PLL sequence to make the PLL stable.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int xvcu_set_pll(struct xvcu_device *xvcu)
> +{
> + u32 lock_status;
> + unsigned long timeout;
> + int ret;
> +
> + ret = xvcu_set_vcu_pll_info(xvcu);
> + if (ret) {
> + dev_err(xvcu->dev, "failed to set pll info\n");
> + return ret;
> + }
> +
> + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> + 1, VCU_PLL_CTRL_BYPASS_MASK,
> + VCU_PLL_CTRL_BYPASS_SHIFT);
> + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> + 1, VCU_PLL_CTRL_RESET_MASK,
> + VCU_PLL_CTRL_RESET_SHIFT);
> + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> + 0, VCU_PLL_CTRL_RESET_MASK,
> + VCU_PLL_CTRL_RESET_SHIFT);
> + /* Defined the timeout for the max time to wait the

comment style.

> + * PLL_STATUS to be locked.
> + */
> + timeout = jiffies + msecs_to_jiffies(2000);
> + do {
> + lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
> + if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
> + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> + 0, VCU_PLL_CTRL_BYPASS_MASK,
> + VCU_PLL_CTRL_BYPASS_SHIFT);
> + return 0;
> + }
> + } while (!time_after(jiffies, timeout));
> +
> + /* PLL is not locked even after the timeout of the 2sec */
> + dev_err(xvcu->dev, "PLL is not locked\n");
> + return -ETIMEDOUT;
> +}
> +
> +/**
> + * xvcu_probe - Probe existence of the logicoreIP
> + * and initialize PLL
> + *
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Return: Returns 0 on success
> + * Negative error code otherwise
> + */
> +static int xvcu_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + struct xvcu_device *xvcu;
> + int ret;
> +
> + xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
> + if (!xvcu)
> + return -ENOMEM;
> +
> + xvcu->dev = &pdev->dev;
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
> + if (!res) {
> + dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
> + return -ENODEV;
> + }
> +
> + xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
> + res->start, resource_size(res));
> + if (!xvcu->vcu_slcr_ba) {
> + dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
> + return -ENOMEM;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
> + if (!res) {
> + dev_err(&pdev->dev, "get logicore memory resource failed.\n");
> + return -ENODEV;
> + }
> +
> + xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
> + res->start, resource_size(res));
> + if (!xvcu->logicore_reg_ba) {
> + dev_err(&pdev->dev, "logicore register mapping failed.\n");
> + return -ENOMEM;
> + }
> +
> + xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
> + if (IS_ERR(xvcu->aclk)) {
> + dev_err(&pdev->dev, "Could not get aclk clock\n");
> + return PTR_ERR(xvcu->aclk);
> + }
> +
> + xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
> + if (IS_ERR(xvcu->pll_ref)) {
> + dev_err(&pdev->dev, "Could not get pll_ref clock\n");
> + return PTR_ERR(xvcu->pll_ref);
> + }
> +
> + ret = clk_prepare_enable(xvcu->aclk);
> + if (ret) {
> + dev_err(&pdev->dev, "aclk clock enable failed\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(xvcu->pll_ref);
> + if (ret) {
> + dev_err(&pdev->dev, "pll_ref clock enable failed\n");
> + goto error_aclk;
> + }
> +
> + /* Do the Gasket isolation and put the VCU out of reset

comment style.

> + * Bit 0 : Gasket isolation
> + * Bit 1 : put VCU out of reset
> + */
> + xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
> +
> + /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
> + ret = xvcu_set_pll(xvcu);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to set the pll\n");
> + goto error_pll_ref;
> + }
> +
> + dev_set_drvdata(&pdev->dev, xvcu);
> +
> + dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
> +
> + return 0;
> +
> +error_pll_ref:
> + clk_disable_unprepare(xvcu->pll_ref);
> +error_aclk:
> + clk_disable_unprepare(xvcu->aclk);
> + return ret;
> +}
> +
> +/**
> + * xvcu_remove - Insert gasket isolation
> + * and disable the clock
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Return: Returns 0 on success
> + * Negative error code otherwise
> + */
> +static int xvcu_remove(struct platform_device *pdev)
> +{
> + struct xvcu_device *xvcu;
> +
> + xvcu = platform_get_drvdata(pdev);
> + if (!xvcu)
> + return -ENODEV;
> +
> + /* Add the the Gasket isolation and put the VCU in reset.

style.

> + */
> + xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> +
> + clk_disable_unprepare(xvcu->pll_ref);
> + clk_disable_unprepare(xvcu->aclk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id xvcu_of_id_table[] = {
> + { .compatible = "xlnx,vcu" },
> + { .compatible = "xlnx,vcu-logicoreip-1.0" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
> +
> +static struct platform_driver xvcu_driver = {
> + .driver = {
> + .name = "xilinx-vcu",
> + .of_match_table = xvcu_of_id_table,
> + },
> + .probe = xvcu_probe,
> + .remove = xvcu_remove,
> +};
> +
> +module_platform_driver(xvcu_driver);
> +
> +MODULE_AUTHOR("Dhaval Shah <[email protected]>");
> +MODULE_DESCRIPTION("Xilinx VCU init Driver");
> +MODULE_LICENSE("GPL v2");
>

The code has several divide operations. Does it build OK for 32-bit target?

--
~Randy

2017-12-15 06:00:59

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH v3 2/2] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Randy,

Thanks a lot for the review.

> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Thursday, December 14, 2017 10:43 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai
> Shah <[email protected]>
> Subject: Re: [PATCH v3 2/2] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver
>
> On 12/13/2017 09:55 PM, Dhaval Shah wrote:
> > Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> > design created. This driver provides the processing system and
> > programmable logic isolation. Set the frequency based on the clock
> > information get from the logicoreIP register set.
> >
> > It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >
> > Signed-off-by: Dhaval Shah <[email protected]>
> > ---
> > Changes since v3:
> > No Changes.
> > Changes since v2:
> > * Removed the "default n" from the Kconfig
> > * More help text added to explain more about the logicoreIP driver
> > * SPDX id is relocated at top of the file with // style comment
> > * Removed the export API and header file and make it a single driver
> > which provides logocoreIP init.
> > * Provide the information in commit message as well for the why driver
> > in drivers/misc.
> >
> > drivers/misc/Kconfig | 15 ++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/xlnx_vcu.c | 629
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 645 insertions(+)
> > create mode 100644 drivers/misc/xlnx_vcu.c
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > f1a5c23..24ea516 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
> > Enable this configuration option to enable the host side test driver
> > for PCI Endpoint.
> >
> > +config XILINX_VCU
> > + tristate "Xilinx VCU logicoreIP Init"
> > + help
>
> Please indent the help text (below) by 2 additional spaces, as documented in
> coding-style.rst.
I will update this.
>
> > + Provides the driver to enable and disable the isolation between the
> > + processing system and programmable logic part by using the
> logicoreIP
> > + register set. This driver also configure the frequency based on
> > +the
>
> configures
>
I will update this.
> > + clock information get from the logicoreIP register set.
>
> drop ^get^
>
I will update this.
> > +
> > + If you say yes here you get support for the logcoreIP.
>
> logicoreIP.
>
> > +
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called xlnx_vcu.
> > +
> > source "drivers/misc/c2port/Kconfig"
> > source "drivers/misc/eeprom/Kconfig"
> > source "drivers/misc/cb710/Kconfig"
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index
> > 5ca5f64..a6bd0b1 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
> > obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> > +obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o
> >
> > lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
> > lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 0000000..41819f0
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,629 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts Dhaval Shah <[email protected]>
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +> +/* Address map for different registers implemented in the VCU
> > +> +LogiCORE IP. */
>
> [snip]
>
>
> > +/**
> > + * xvcu_read - Read from the VCU register space
> > + * @iomem: vcu reg space base address
> > + * @offset: vcu reg offset from base
> > + *
> > + * Return: Returns 32bit value from VCU register specified
> > + *
> > + */
> > +static u32 xvcu_read(void __iomem *iomem, u32 offset)
>
> You could inline the read() and write() functions...
>
I will check and will update if required.
> > +{
> > + return ioread32(iomem + offset);
> > +}
> > +
> > +/**
> > + * xvcu_write - Write to the VCU register space
> > + * @iomem: vcu reg space base address
> > + * @offset: vcu reg offset from base
> > + * @value: Value to write
> > + */
> > +static void xvcu_write(void __iomem *iomem, u32 offset, u32 value) {
> > + iowrite32(value, iomem + offset);
> > +}
> > +
> > +/**
> > + * xvcu_write_field_reg - Write to the vcu reg field
> > + * @iomem: vcu reg space base address
> > + * @offset: vcu reg offset from base
> > + * @field: vcu reg field to write to
> > + * @mask: vcu reg mask
> > + * @shift: vcu reg number of bits to shift the bitfield
> > + */
> > +static void xvcu_write_field_reg(void __iomem *iomem, int offset,
> > + u32 field, u32 mask, int shift)
> > +{
> > + u32 val = xvcu_read(iomem, offset);
> > +
> > + val &= ~(mask << shift);
> > + val |= (field & mask) << shift;
> > +
> > + xvcu_write(iomem, offset, val);
> > +}
> > +
> > +/**
> > + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> > + * @xvcu: Pointer to the xvcu_device structure
> > + *
> > + * Programming the VCU PLL based on the user configuration
> > + * (ref clock freq, core clock freq, mcu clock freq).
> > + * Core clock frequency has higher priority than mcu clock frequency
> > + * Errors in following cases
> > + * - When mcu or clock clock get from logicoreIP is 0
> > + * - When VCU PLL DIV related bits value other than 1
> > + * - When proper data not found for given data
> > + * - When sis570_1 clocksource related operation failed
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> > + u32 refclk, coreclk, mcuclk, inte, deci;
> > + u32 divisor_mcu, divisor_core, fvco;
> > + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> > + u32 cfg_val, mod, ctrl;
> > + int ret;
> > + unsigned int i;
> > + const struct xvcu_pll_cfg *found = NULL;
> > +
> > + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> > + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> > + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> > + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> > + if (!mcuclk || !coreclk) {
> > + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> > + return -EINVAL;
> > + }
> > +
> > + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> > + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> > + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> > + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> > +
> > + clk_disable_unprepare(xvcu->pll_ref);
> > + ret = clk_set_rate(xvcu->pll_ref, refclk);
> > + if (ret)
> > + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> > +
> > + ret = clk_prepare_enable(xvcu->pll_ref);
> > + if (ret) {
> > + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> > + return ret;
> > + }
> > +
> > + refclk = clk_get_rate(xvcu->pll_ref);
> > +
> > + /* The divide-by-2 should be always enabled (==1)
>
> multi-line comment style:
> /*
> * The divide-by-2 should always be enabled (==1)
>
I will update this.
> > + * to meet the timing in the design.
> > + * Otherwise, it's an error
> > + */
> > + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> > + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> > + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> > + if (clkoutdiv != 1) {
> > + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> > + return -EINVAL;
> > + }
> > +
> > + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
> > + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> > +
> > + fvco = cfg->fbdiv * refclk;
> > + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> > + pll_clk = fvco / VCU_PLL_DIV2;
> > + if (fvco % VCU_PLL_DIV2 != 0)
> > + pll_clk++;
> > + mod = pll_clk % coreclk;
> > + if (mod < LIMIT) {
> > + divisor_core = pll_clk / coreclk;
> > + } else if (coreclk - mod < LIMIT) {
> > + divisor_core = pll_clk / coreclk;
> > + divisor_core++;
> > + } else {
> > + continue;
> > + }
> > + if (divisor_core >= DIVISOR_MIN &&
> > + divisor_core <= DIVISOR_MAX) {
> > + found = cfg;
> > + divisor_mcu = pll_clk / mcuclk;
> > + mod = pll_clk % mcuclk;
> > + if (mcuclk - mod < LIMIT)
> > + divisor_mcu++;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + if (!found) {
> > + dev_err(xvcu->dev, "Invalid clock combination.\n");
> > + return -EINVAL;
> > + }
> > +
> > + xvcu->coreclk = pll_clk / divisor_core;
> > + mcuclk = pll_clk / divisor_mcu;
> > + dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> > + dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> > + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> > +
> > + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK <<
> VCU_PLL_CTRL_FBDIV_SHIFT);
> > + vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> > + VCU_PLL_CTRL_FBDIV_SHIFT;
> > + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
> > + VCU_PLL_CTRL_POR_IN_SHIFT);
> > + vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT &
> VCU_PLL_CTRL_POR_IN_MASK) <<
> > + VCU_PLL_CTRL_POR_IN_SHIFT;
> > + vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
> > + VCU_PLL_CTRL_PWR_POR_SHIFT);
> > + vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT &
> VCU_PLL_CTRL_PWR_POR_MASK) <<
> > + VCU_PLL_CTRL_PWR_POR_SHIFT;
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
> > +
> > + /* Set divisor for the core and mcu clock */
> > + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
> > + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> > + ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> > + VCU_PLL_DIVISOR_SHIFT;
> > + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> > + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) <<
> VCU_SRCSEL_SHIFT;
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
> > +
> > + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
> > + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> > + ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> > + VCU_PLL_DIVISOR_SHIFT;
> > + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> > + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) <<
> VCU_SRCSEL_SHIFT;
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
> > +
> > + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
> > + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> > + ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) <<
> VCU_PLL_DIVISOR_SHIFT;
> > + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> > + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) <<
> VCU_SRCSEL_SHIFT;
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
> > +
> > + ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
> > + ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> > + ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) <<
> VCU_PLL_DIVISOR_SHIFT;
> > + ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> > + ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) <<
> VCU_SRCSEL_SHIFT;
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
> > +
> > + /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
> > + cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
> > + (found->cp << VCU_PLL_CFG_CP_SHIFT) |
> > + (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
> > + (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
> > + (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
> > + xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xvcu_set_pll - PLL init sequence
> > + * @xvcu: Pointer to the xvcu_device structure
> > + *
> > + * Call the api to set the PLL info and once that is done then
> > + * init the PLL sequence to make the PLL stable.
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +static int xvcu_set_pll(struct xvcu_device *xvcu) {
> > + u32 lock_status;
> > + unsigned long timeout;
> > + int ret;
> > +
> > + ret = xvcu_set_vcu_pll_info(xvcu);
> > + if (ret) {
> > + dev_err(xvcu->dev, "failed to set pll info\n");
> > + return ret;
> > + }
> > +
> > + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> > + 1, VCU_PLL_CTRL_BYPASS_MASK,
> > + VCU_PLL_CTRL_BYPASS_SHIFT);
> > + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> > + 1, VCU_PLL_CTRL_RESET_MASK,
> > + VCU_PLL_CTRL_RESET_SHIFT);
> > + xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> > + 0, VCU_PLL_CTRL_RESET_MASK,
> > + VCU_PLL_CTRL_RESET_SHIFT);
> > + /* Defined the timeout for the max time to wait the
>
> comment style.
>
I will update this.
> > + * PLL_STATUS to be locked.
> > + */
> > + timeout = jiffies + msecs_to_jiffies(2000);
> > + do {
> > + lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
> > + if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
> > + xvcu_write_field_reg(xvcu->vcu_slcr_ba,
> VCU_PLL_CTRL,
> > + 0, VCU_PLL_CTRL_BYPASS_MASK,
> > + VCU_PLL_CTRL_BYPASS_SHIFT);
> > + return 0;
> > + }
> > + } while (!time_after(jiffies, timeout));
> > +
> > + /* PLL is not locked even after the timeout of the 2sec */
> > + dev_err(xvcu->dev, "PLL is not locked\n");
> > + return -ETIMEDOUT;
> > +}
> > +
> > +/**
> > + * xvcu_probe - Probe existence of the logicoreIP
> > + * and initialize PLL
> > + *
> > + * @pdev: Pointer to the platform_device structure
> > + *
> > + * Return: Returns 0 on success
> > + * Negative error code otherwise
> > + */
> > +static int xvcu_probe(struct platform_device *pdev) {
> > + struct resource *res;
> > + struct xvcu_device *xvcu;
> > + int ret;
> > +
> > + xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
> > + if (!xvcu)
> > + return -ENOMEM;
> > +
> > + xvcu->dev = &pdev->dev;
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "vcu_slcr");
> > + if (!res) {
> > + dev_err(&pdev->dev, "get vcu_slcr memory resource
> failed.\n");
> > + return -ENODEV;
> > + }
> > +
> > + xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
> > + res->start, resource_size(res));
> > + if (!xvcu->vcu_slcr_ba) {
> > + dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
> > + return -ENOMEM;
> > + }
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "logicore");
> > + if (!res) {
> > + dev_err(&pdev->dev, "get logicore memory resource
> failed.\n");
> > + return -ENODEV;
> > + }
> > +
> > + xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
> > + res->start, resource_size(res));
> > + if (!xvcu->logicore_reg_ba) {
> > + dev_err(&pdev->dev, "logicore register mapping failed.\n");
> > + return -ENOMEM;
> > + }
> > +
> > + xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
> > + if (IS_ERR(xvcu->aclk)) {
> > + dev_err(&pdev->dev, "Could not get aclk clock\n");
> > + return PTR_ERR(xvcu->aclk);
> > + }
> > +
> > + xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
> > + if (IS_ERR(xvcu->pll_ref)) {
> > + dev_err(&pdev->dev, "Could not get pll_ref clock\n");
> > + return PTR_ERR(xvcu->pll_ref);
> > + }
> > +
> > + ret = clk_prepare_enable(xvcu->aclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "aclk clock enable failed\n");
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(xvcu->pll_ref);
> > + if (ret) {
> > + dev_err(&pdev->dev, "pll_ref clock enable failed\n");
> > + goto error_aclk;
> > + }
> > +
> > + /* Do the Gasket isolation and put the VCU out of reset
>
> comment style.
>
I will update this.
> > + * Bit 0 : Gasket isolation
> > + * Bit 1 : put VCU out of reset
> > + */
> > + xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT,
> > +VCU_GASKET_VALUE);
> > +
> > + /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
> > + ret = xvcu_set_pll(xvcu);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Failed to set the pll\n");
> > + goto error_pll_ref;
> > + }
> > +
> > + dev_set_drvdata(&pdev->dev, xvcu);
> > +
> > + dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
> > +
> > + return 0;
> > +
> > +error_pll_ref:
> > + clk_disable_unprepare(xvcu->pll_ref);
> > +error_aclk:
> > + clk_disable_unprepare(xvcu->aclk);
> > + return ret;
> > +}
> > +
> > +/**
> > + * xvcu_remove - Insert gasket isolation
> > + * and disable the clock
> > + * @pdev: Pointer to the platform_device structure
> > + *
> > + * Return: Returns 0 on success
> > + * Negative error code otherwise
> > + */
> > +static int xvcu_remove(struct platform_device *pdev) {
> > + struct xvcu_device *xvcu;
> > +
> > + xvcu = platform_get_drvdata(pdev);
> > + if (!xvcu)
> > + return -ENODEV;
> > +
> > + /* Add the the Gasket isolation and put the VCU in reset.
>
> style.
>
I will update this.
> > + */
> > + xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> > +
> > + clk_disable_unprepare(xvcu->pll_ref);
> > + clk_disable_unprepare(xvcu->aclk);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id xvcu_of_id_table[] = {
> > + { .compatible = "xlnx,vcu" },
> > + { .compatible = "xlnx,vcu-logicoreip-1.0" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
> > +
> > +static struct platform_driver xvcu_driver = {
> > + .driver = {
> > + .name = "xilinx-vcu",
> > + .of_match_table = xvcu_of_id_table,
> > + },
> > + .probe = xvcu_probe,
> > + .remove = xvcu_remove,
> > +};
> > +
> > +module_platform_driver(xvcu_driver);
> > +
> > +MODULE_AUTHOR("Dhaval Shah <[email protected]>");
> > +MODULE_DESCRIPTION("Xilinx VCU init Driver"); MODULE_LICENSE("GPL
> > +v2");
> >
>
> The code has several divide operations. Does it build OK for 32-bit target?
>
Yes. I have verified on both 32 bit and 64 bit target.
> --
> ~Randy

Thanks & Regards,
Dhaval

2017-12-15 07:24:51

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v4 0/2] Documentation and driver of logicoreIP

1st patch provide Device Tree binding document for logicoreIP
2nd patch provide the xlnx_vcu logicoreIP driver, Kconfig changes
and Makefile changes for the driver.

Dhaval Shah (2):
Documentation: devicetree: Add DT bindings to xlnx_vcu driver
misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 +
drivers/misc/Kconfig | 15 +
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 631 +++++++++++++++++++++
4 files changed, 678 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt
create mode 100644 drivers/misc/xlnx_vcu.c

--
2.7.4

2017-12-15 07:25:02

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v4 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

From: Dhaval Shah <[email protected]>

Add Device Tree binding document for logicoreIP. This logicoreIP
provides the isolation between the processing system and
programmable logic. Also provides the clock related information.

Signed-off-by: Dhaval Shah <[email protected]>
---
Chnages since v3:
No Changes.
Chnages since v3:
* Use "dt-bindings: misc: ..." for the subject.
Changes since v2:
* Describe the h/w
* compatible string is updated to make it more specific
based on the logicoreIP version.
* Removed that encoder and decoder child nodes and relatd properties as that
will be a separate driver and dts nodes. other team is working on that.
* Updated to use as a single driver.

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt

diff --git a/Documentation/devicetree/bindings/misc/xlnx,vcu.txt b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
new file mode 100644
index 0000000..6786d67
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
@@ -0,0 +1,31 @@
+LogicoreIP designed compatible with Xilinx ZYNQ family.
+-------------------------------------------------------
+
+General concept
+---------------
+
+LogicoreIP design to provide the isolation between processing system
+and programmable logic. Also provides the list of register set to configure
+the frequency.
+
+Required properties:
+- compatible: shall be one of:
+ "xlnx,vcu"
+ "xlnx,vcu-logicoreip-1.0"
+- reg, reg-names: There are two sets of registers need to provide.
+ 1. vcu slcr
+ 2. Logicore
+ reg-names should contain name for the each register sequence.
+- clocks: phandle for aclk and pll_ref clocksource
+- clock-names: The identification string, "aclk", is always required for
+ the axi clock. "pll_ref" is required for pll.
+Example:
+
+ xlnx_vcu: vcu@a0040000 {
+ compatible = "xlnx,vcu-logicoreip-1.0";
+ reg = <0x0 0xa0040000 0x0 0x1000>,
+ <0x0 0xa0041000 0x0 0x1000>;
+ reg-names = "vcu_slcr", "logicore";
+ clocks = <&si570_1>, <&clkc 71>;
+ clock-names = "pll_ref", "aclk";
+ };
--
2.7.4

2017-12-15 07:25:14

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

Xilinx ZYNQMP logicoreIP Init driver is based on the new
LogiCoreIP design created. This driver provides the processing system
and programmable logic isolation. Set the frequency based on the clock
information get from the logicoreIP register set.

It is put in drivers/misc as there is no subsystem for this logicoreIP.

Signed-off-by: Dhaval Shah <[email protected]>
---
Changes since v4:
* Indent the help text (below) by 2 additional spaces, as documented in coding-style.rst
* Spell check are resolved as per the review comment.
* inline the read() and write() functions..
* multi-line comment style
* Updated subject line prefix
Changes since v3:
No Changes.
Changes since v2:
* Removed the "default n" from the Kconfig
* More help text added to explain more about the logicoreIP driver
* SPDX id is relocated at top of the file with // style comment
* Removed the export API and header file and make it a single driver
which provides logocoreIP init.
* Provide the information in commit message as well for the why driver
in drivers/misc.

drivers/misc/Kconfig | 15 ++
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 631 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 647 insertions(+)
create mode 100644 drivers/misc/xlnx_vcu.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f1a5c23..e679936 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
Enable this configuration option to enable the host side test driver
for PCI Endpoint.

+config XILINX_VCU
+ tristate "Xilinx VCU logicoreIP Init"
+ help
+ Provides the driver to enable and disable the isolation between the
+ processing system and programmable logic part by using the logicoreIP
+ register set. This driver also configures the frequency based on the
+ clock information from the logicoreIP register set.
+
+ If you say yes here you get support for the logicoreIP.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called xlnx_vcu.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 5ca5f64..a6bd0b1 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
+obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o

lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
new file mode 100644
index 0000000..f489d34
--- /dev/null
+++ b/drivers/misc/xlnx_vcu.c
@@ -0,0 +1,631 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+/* Address map for different registers implemented in the VCU LogiCORE IP. */
+#define VCU_ECODER_ENABLE 0x00
+#define VCU_DECODER_ENABLE 0x04
+#define VCU_MEMORY_DEPTH 0x08
+#define VCU_ENC_COLOR_DEPTH 0x0c
+#define VCU_ENC_VERTICAL_RANGE 0x10
+#define VCU_ENC_FRAME_SIZE_X 0x14
+#define VCU_ENC_FRAME_SIZE_Y 0x18
+#define VCU_ENC_COLOR_FORMAT 0x1c
+#define VCU_ENC_FPS 0x20
+#define VCU_MCU_CLK 0x24
+#define VCU_CORE_CLK 0x28
+#define VCU_PLL_BYPASS 0x2c
+#define VCU_ENC_CLK 0x30
+#define VCU_PLL_CLK 0x34
+#define VCU_ENC_VIDEO_STANDARD 0x38
+#define VCU_STATUS 0x3c
+#define VCU_AXI_ENC_CLK 0x40
+#define VCU_AXI_DEC_CLK 0x44
+#define VCU_AXI_MCU_CLK 0x48
+#define VCU_DEC_VIDEO_STANDARD 0x4c
+#define VCU_DEC_FRAME_SIZE_X 0x50
+#define VCU_DEC_FRAME_SIZE_Y 0x54
+#define VCU_DEC_FPS 0x58
+#define VCU_BUFFER_B_FRAME 0x5c
+#define VCU_WPP_EN 0x60
+#define VCU_PLL_CLK_DEC 0x64
+#define VCU_GASKET_INIT 0x74
+#define VCU_GASKET_VALUE 0x03
+
+/* vcu slcr registers, bitmask and shift */
+#define VCU_PLL_CTRL 0x24
+#define VCU_PLL_CTRL_RESET_MASK 0x01
+#define VCU_PLL_CTRL_RESET_SHIFT 0
+#define VCU_PLL_CTRL_BYPASS_MASK 0x01
+#define VCU_PLL_CTRL_BYPASS_SHIFT 3
+#define VCU_PLL_CTRL_FBDIV_MASK 0x7f
+#define VCU_PLL_CTRL_FBDIV_SHIFT 8
+#define VCU_PLL_CTRL_POR_IN_MASK 0x01
+#define VCU_PLL_CTRL_POR_IN_SHIFT 1
+#define VCU_PLL_CTRL_PWR_POR_MASK 0x01
+#define VCU_PLL_CTRL_PWR_POR_SHIFT 2
+#define VCU_PLL_CTRL_CLKOUTDIV_MASK 0x03
+#define VCU_PLL_CTRL_CLKOUTDIV_SHIFT 16
+#define VCU_PLL_CTRL_DEFAULT 0
+#define VCU_PLL_DIV2 2
+
+#define VCU_PLL_CFG 0x28
+#define VCU_PLL_CFG_RES_MASK 0x0f
+#define VCU_PLL_CFG_RES_SHIFT 0
+#define VCU_PLL_CFG_CP_MASK 0x0f
+#define VCU_PLL_CFG_CP_SHIFT 5
+#define VCU_PLL_CFG_LFHF_MASK 0x03
+#define VCU_PLL_CFG_LFHF_SHIFT 10
+#define VCU_PLL_CFG_LOCK_CNT_MASK 0x03ff
+#define VCU_PLL_CFG_LOCK_CNT_SHIFT 13
+#define VCU_PLL_CFG_LOCK_DLY_MASK 0x7f
+#define VCU_PLL_CFG_LOCK_DLY_SHIFT 25
+#define VCU_ENC_CORE_CTRL 0x30
+#define VCU_ENC_MCU_CTRL 0x34
+#define VCU_DEC_CORE_CTRL 0x38
+#define VCU_DEC_MCU_CTRL 0x3c
+#define VCU_PLL_DIVISOR_MASK 0x3f
+#define VCU_PLL_DIVISOR_SHIFT 4
+#define VCU_SRCSEL_MASK 0x01
+#define VCU_SRCSEL_SHIFT 0
+#define VCU_SRCSEL_PLL 1
+
+#define VCU_PLL_STATUS 0x60
+#define VCU_PLL_STATUS_LOCK_STATUS_MASK 0x01
+
+#define MHZ 1000000
+#define FVCO_MIN (1500U * MHZ)
+#define FVCO_MAX (3000U * MHZ)
+#define DIVISOR_MIN 0
+#define DIVISOR_MAX 63
+#define FRAC 100
+#define LIMIT (10 * MHZ)
+
+/**
+ * struct xvcu_device - Xilinx VCU init device structure
+ * @dev: Platform device
+ * @pll_ref: pll ref clock source
+ * @aclk: axi clock source
+ * @logicore_reg_ba: logicore reg base address
+ * @vcu_slcr_ba: vcu_slcr Register base address
+ * @coreclk: core clock frequency
+ */
+struct xvcu_device {
+ struct device *dev;
+ struct clk *pll_ref;
+ struct clk *aclk;
+ void __iomem *logicore_reg_ba;
+ void __iomem *vcu_slcr_ba;
+ u32 coreclk;
+};
+
+/**
+ * struct xvcu_pll_cfg - Helper data
+ * @fbdiv: The integer portion of the feedback divider to the PLL
+ * @cp: PLL charge pump control
+ * @res: PLL loop filter resistor control
+ * @lfhf: PLL loop filter high frequency capacitor control
+ * @lock_dly: Lock circuit configuration settings for lock windowsize
+ * @lock_cnt: Lock circuit counter setting
+ */
+struct xvcu_pll_cfg {
+ u32 fbdiv;
+ u32 cp;
+ u32 res;
+ u32 lfhf;
+ u32 lock_dly;
+ u32 lock_cnt;
+};
+
+static const struct xvcu_pll_cfg xvcu_pll_cfg[] = {
+ { 25, 3, 10, 3, 63, 1000 },
+ { 26, 3, 10, 3, 63, 1000 },
+ { 27, 4, 6, 3, 63, 1000 },
+ { 28, 4, 6, 3, 63, 1000 },
+ { 29, 4, 6, 3, 63, 1000 },
+ { 30, 4, 6, 3, 63, 1000 },
+ { 31, 6, 1, 3, 63, 1000 },
+ { 32, 6, 1, 3, 63, 1000 },
+ { 33, 4, 10, 3, 63, 1000 },
+ { 34, 5, 6, 3, 63, 1000 },
+ { 35, 5, 6, 3, 63, 1000 },
+ { 36, 5, 6, 3, 63, 1000 },
+ { 37, 5, 6, 3, 63, 1000 },
+ { 38, 5, 6, 3, 63, 975 },
+ { 39, 3, 12, 3, 63, 950 },
+ { 40, 3, 12, 3, 63, 925 },
+ { 41, 3, 12, 3, 63, 900 },
+ { 42, 3, 12, 3, 63, 875 },
+ { 43, 3, 12, 3, 63, 850 },
+ { 44, 3, 12, 3, 63, 850 },
+ { 45, 3, 12, 3, 63, 825 },
+ { 46, 3, 12, 3, 63, 800 },
+ { 47, 3, 12, 3, 63, 775 },
+ { 48, 3, 12, 3, 63, 775 },
+ { 49, 3, 12, 3, 63, 750 },
+ { 50, 3, 12, 3, 63, 750 },
+ { 51, 3, 2, 3, 63, 725 },
+ { 52, 3, 2, 3, 63, 700 },
+ { 53, 3, 2, 3, 63, 700 },
+ { 54, 3, 2, 3, 63, 675 },
+ { 55, 3, 2, 3, 63, 675 },
+ { 56, 3, 2, 3, 63, 650 },
+ { 57, 3, 2, 3, 63, 650 },
+ { 58, 3, 2, 3, 63, 625 },
+ { 59, 3, 2, 3, 63, 625 },
+ { 60, 3, 2, 3, 63, 625 },
+ { 61, 3, 2, 3, 63, 600 },
+ { 62, 3, 2, 3, 63, 600 },
+ { 63, 3, 2, 3, 63, 600 },
+ { 64, 3, 2, 3, 63, 600 },
+ { 65, 3, 2, 3, 63, 600 },
+ { 66, 3, 2, 3, 63, 600 },
+ { 67, 3, 2, 3, 63, 600 },
+ { 68, 3, 2, 3, 63, 600 },
+ { 69, 3, 2, 3, 63, 600 },
+ { 70, 3, 2, 3, 63, 600 },
+ { 71, 3, 2, 3, 63, 600 },
+ { 72, 3, 2, 3, 63, 600 },
+ { 73, 3, 2, 3, 63, 600 },
+ { 74, 3, 2, 3, 63, 600 },
+ { 75, 3, 2, 3, 63, 600 },
+ { 76, 3, 2, 3, 63, 600 },
+ { 77, 3, 2, 3, 63, 600 },
+ { 78, 3, 2, 3, 63, 600 },
+ { 79, 3, 2, 3, 63, 600 },
+ { 80, 3, 2, 3, 63, 600 },
+ { 81, 3, 2, 3, 63, 600 },
+ { 82, 3, 2, 3, 63, 600 },
+ { 83, 4, 2, 3, 63, 600 },
+ { 84, 4, 2, 3, 63, 600 },
+ { 85, 4, 2, 3, 63, 600 },
+ { 86, 4, 2, 3, 63, 600 },
+ { 87, 4, 2, 3, 63, 600 },
+ { 88, 4, 2, 3, 63, 600 },
+ { 89, 4, 2, 3, 63, 600 },
+ { 90, 4, 2, 3, 63, 600 },
+ { 91, 4, 2, 3, 63, 600 },
+ { 92, 4, 2, 3, 63, 600 },
+ { 93, 4, 2, 3, 63, 600 },
+ { 94, 4, 2, 3, 63, 600 },
+ { 95, 4, 2, 3, 63, 600 },
+ { 96, 4, 2, 3, 63, 600 },
+ { 97, 4, 2, 3, 63, 600 },
+ { 98, 4, 2, 3, 63, 600 },
+ { 99, 4, 2, 3, 63, 600 },
+ { 100, 4, 2, 3, 63, 600 },
+ { 101, 4, 2, 3, 63, 600 },
+ { 102, 4, 2, 3, 63, 600 },
+ { 103, 5, 2, 3, 63, 600 },
+ { 104, 5, 2, 3, 63, 600 },
+ { 105, 5, 2, 3, 63, 600 },
+ { 106, 5, 2, 3, 63, 600 },
+ { 107, 3, 4, 3, 63, 600 },
+ { 108, 3, 4, 3, 63, 600 },
+ { 109, 3, 4, 3, 63, 600 },
+ { 110, 3, 4, 3, 63, 600 },
+ { 111, 3, 4, 3, 63, 600 },
+ { 112, 3, 4, 3, 63, 600 },
+ { 113, 3, 4, 3, 63, 600 },
+ { 114, 3, 4, 3, 63, 600 },
+ { 115, 3, 4, 3, 63, 600 },
+ { 116, 3, 4, 3, 63, 600 },
+ { 117, 3, 4, 3, 63, 600 },
+ { 118, 3, 4, 3, 63, 600 },
+ { 119, 3, 4, 3, 63, 600 },
+ { 120, 3, 4, 3, 63, 600 },
+ { 121, 3, 4, 3, 63, 600 },
+ { 122, 3, 4, 3, 63, 600 },
+ { 123, 3, 4, 3, 63, 600 },
+ { 124, 3, 4, 3, 63, 600 },
+ { 125, 3, 4, 3, 63, 600 },
+};
+
+/**
+ * xvcu_read - Read from the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ *
+ * Return: Returns 32bit value from VCU register specified
+ *
+ */
+static inline u32 xvcu_read(void __iomem *iomem, u32 offset)
+{
+ return ioread32(iomem + offset);
+}
+
+/**
+ * xvcu_write - Write to the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @value: Value to write
+ */
+static inline void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
+{
+ iowrite32(value, iomem + offset);
+}
+
+/**
+ * xvcu_write_field_reg - Write to the vcu reg field
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @field: vcu reg field to write to
+ * @mask: vcu reg mask
+ * @shift: vcu reg number of bits to shift the bitfield
+ */
+static void xvcu_write_field_reg(void __iomem *iomem, int offset,
+ u32 field, u32 mask, int shift)
+{
+ u32 val = xvcu_read(iomem, offset);
+
+ val &= ~(mask << shift);
+ val |= (field & mask) << shift;
+
+ xvcu_write(iomem, offset, val);
+}
+
+/**
+ * xvcu_set_vcu_pll_info - Set the VCU PLL info
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Programming the VCU PLL based on the user configuration
+ * (ref clock freq, core clock freq, mcu clock freq).
+ * Core clock frequency has higher priority than mcu clock frequency
+ * Errors in following cases
+ * - When mcu or clock clock get from logicoreIP is 0
+ * - When VCU PLL DIV related bits value other than 1
+ * - When proper data not found for given data
+ * - When sis570_1 clocksource related operation failed
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
+{
+ u32 refclk, coreclk, mcuclk, inte, deci;
+ u32 divisor_mcu, divisor_core, fvco;
+ u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
+ u32 cfg_val, mod, ctrl;
+ int ret;
+ unsigned int i;
+ const struct xvcu_pll_cfg *found = NULL;
+
+ inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
+ deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
+ coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
+ mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
+ if (!mcuclk || !coreclk) {
+ dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
+ return -EINVAL;
+ }
+
+ refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
+ dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
+ dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ ret = clk_set_rate(xvcu->pll_ref, refclk);
+ if (ret)
+ dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
+ return ret;
+ }
+
+ refclk = clk_get_rate(xvcu->pll_ref);
+
+ /*
+ * The divide-by-2 should be always enabled (==1)
+ * to meet the timing in the design.
+ * Otherwise, it's an error
+ */
+ vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
+ clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
+ clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
+ if (clkoutdiv != 1) {
+ dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
+ return -EINVAL;
+ }
+
+ for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
+ const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
+
+ fvco = cfg->fbdiv * refclk;
+ if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
+ pll_clk = fvco / VCU_PLL_DIV2;
+ if (fvco % VCU_PLL_DIV2 != 0)
+ pll_clk++;
+ mod = pll_clk % coreclk;
+ if (mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ } else if (coreclk - mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ divisor_core++;
+ } else {
+ continue;
+ }
+ if (divisor_core >= DIVISOR_MIN &&
+ divisor_core <= DIVISOR_MAX) {
+ found = cfg;
+ divisor_mcu = pll_clk / mcuclk;
+ mod = pll_clk % mcuclk;
+ if (mcuclk - mod < LIMIT)
+ divisor_mcu++;
+ break;
+ }
+ }
+ }
+
+ if (!found) {
+ dev_err(xvcu->dev, "Invalid clock combination.\n");
+ return -EINVAL;
+ }
+
+ xvcu->coreclk = pll_clk / divisor_core;
+ mcuclk = pll_clk / divisor_mcu;
+ dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
+ dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
+
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
+ vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
+ VCU_PLL_CTRL_FBDIV_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
+ VCU_PLL_CTRL_POR_IN_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
+ VCU_PLL_CTRL_POR_IN_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+ /* Set divisor for the core and mcu clock */
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
+
+ /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
+ cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
+ (found->cp << VCU_PLL_CFG_CP_SHIFT) |
+ (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
+ (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
+ (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
+
+ return 0;
+}
+
+/**
+ * xvcu_set_pll - PLL init sequence
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Call the api to set the PLL info and once that is done then
+ * init the PLL sequence to make the PLL stable.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_pll(struct xvcu_device *xvcu)
+{
+ u32 lock_status;
+ unsigned long timeout;
+ int ret;
+
+ ret = xvcu_set_vcu_pll_info(xvcu);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to set pll info\n");
+ return ret;
+ }
+
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ /*
+ * Defined the timeout for the max time to wait the
+ * PLL_STATUS to be locked.
+ */
+ timeout = jiffies + msecs_to_jiffies(2000);
+ do {
+ lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
+ if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ return 0;
+ }
+ } while (!time_after(jiffies, timeout));
+
+ /* PLL is not locked even after the timeout of the 2sec */
+ dev_err(xvcu->dev, "PLL is not locked\n");
+ return -ETIMEDOUT;
+}
+
+/**
+ * xvcu_probe - Probe existence of the logicoreIP
+ * and initialize PLL
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct xvcu_device *xvcu;
+ int ret;
+
+ xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
+ if (!xvcu)
+ return -ENOMEM;
+
+ xvcu->dev = &pdev->dev;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
+ if (!res) {
+ dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->vcu_slcr_ba) {
+ dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
+ if (!res) {
+ dev_err(&pdev->dev, "get logicore memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->logicore_reg_ba) {
+ dev_err(&pdev->dev, "logicore register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
+ if (IS_ERR(xvcu->aclk)) {
+ dev_err(&pdev->dev, "Could not get aclk clock\n");
+ return PTR_ERR(xvcu->aclk);
+ }
+
+ xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
+ if (IS_ERR(xvcu->pll_ref)) {
+ dev_err(&pdev->dev, "Could not get pll_ref clock\n");
+ return PTR_ERR(xvcu->pll_ref);
+ }
+
+ ret = clk_prepare_enable(xvcu->aclk);
+ if (ret) {
+ dev_err(&pdev->dev, "aclk clock enable failed\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(&pdev->dev, "pll_ref clock enable failed\n");
+ goto error_aclk;
+ }
+
+ /*
+ * Do the Gasket isolation and put the VCU out of reset
+ * Bit 0 : Gasket isolation
+ * Bit 1 : put VCU out of reset
+ */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
+
+ /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
+ ret = xvcu_set_pll(xvcu);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to set the pll\n");
+ goto error_pll_ref;
+ }
+
+ dev_set_drvdata(&pdev->dev, xvcu);
+
+ dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
+
+ return 0;
+
+error_pll_ref:
+ clk_disable_unprepare(xvcu->pll_ref);
+error_aclk:
+ clk_disable_unprepare(xvcu->aclk);
+ return ret;
+}
+
+/**
+ * xvcu_remove - Insert gasket isolation
+ * and disable the clock
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_remove(struct platform_device *pdev)
+{
+ struct xvcu_device *xvcu;
+
+ xvcu = platform_get_drvdata(pdev);
+ if (!xvcu)
+ return -ENODEV;
+
+ /* Add the the Gasket isolation and put the VCU in reset. */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ clk_disable_unprepare(xvcu->aclk);
+
+ return 0;
+}
+
+static const struct of_device_id xvcu_of_id_table[] = {
+ { .compatible = "xlnx,vcu" },
+ { .compatible = "xlnx,vcu-logicoreip-1.0" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
+
+static struct platform_driver xvcu_driver = {
+ .driver = {
+ .name = "xilinx-vcu",
+ .of_match_table = xvcu_of_id_table,
+ },
+ .probe = xvcu_probe,
+ .remove = xvcu_remove,
+};
+
+module_platform_driver(xvcu_driver);
+
+MODULE_AUTHOR("Dhaval Shah <[email protected]>");
+MODULE_DESCRIPTION("Xilinx VCU init Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2017-12-15 13:26:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah <[email protected]> wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
>
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>
> Signed-off-by: Dhaval Shah <[email protected]>

After giving this some more thought, I'd suggest you move the driver to
drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
it merged by Michal Simek as a driver patch that will go through arm-soc.

Arnd

2017-12-16 18:20:47

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

On Thu, Dec 14, 2017 at 11:24:15PM -0800, Dhaval Shah wrote:
> From: Dhaval Shah <[email protected]>
>
> Add Device Tree binding document for logicoreIP. This logicoreIP
> provides the isolation between the processing system and
> programmable logic. Also provides the clock related information.
>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
> Chnages since v3:
> No Changes.
> Chnages since v3:
> * Use "dt-bindings: misc: ..." for the subject.
> Changes since v2:
> * Describe the h/w
> * compatible string is updated to make it more specific
> based on the logicoreIP version.
> * Removed that encoder and decoder child nodes and relatd properties as that
> will be a separate driver and dts nodes. other team is working on that.
> * Updated to use as a single driver.
>
> .../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt

Reviewed-by: Rob Herring <[email protected]>

2017-12-16 22:18:15

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
>
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
>
> drivers/misc/Kconfig | 15 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/xlnx_vcu.c | 631 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 647 insertions(+)
> create mode 100644 drivers/misc/xlnx_vcu.c

> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 0000000..f489d34
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,631 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts Dhaval Shah <[email protected]>
> + */
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>

[snip]


> +/**
> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> + * @xvcu: Pointer to the xvcu_device structure
> + *
> + * Programming the VCU PLL based on the user configuration
> + * (ref clock freq, core clock freq, mcu clock freq).
> + * Core clock frequency has higher priority than mcu clock frequency
> + * Errors in following cases
> + * - When mcu or clock clock get from logicoreIP is 0
> + * - When VCU PLL DIV related bits value other than 1
> + * - When proper data not found for given data
> + * - When sis570_1 clocksource related operation failed
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
> +{
> + u32 refclk, coreclk, mcuclk, inte, deci;
> + u32 divisor_mcu, divisor_core, fvco;
> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> + u32 cfg_val, mod, ctrl;
> + int ret;
> + unsigned int i;
> + const struct xvcu_pll_cfg *found = NULL;
> +
> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> + if (!mcuclk || !coreclk) {
> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> + return -EINVAL;
> + }
> +
> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> +
> + clk_disable_unprepare(xvcu->pll_ref);
> + ret = clk_set_rate(xvcu->pll_ref, refclk);
> + if (ret)
> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> +
> + ret = clk_prepare_enable(xvcu->pll_ref);
> + if (ret) {
> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> + return ret;
> + }
> +
> + refclk = clk_get_rate(xvcu->pll_ref);
> +
> + /*
> + * The divide-by-2 should be always enabled (==1)
> + * to meet the timing in the design.
> + * Otherwise, it's an error
> + */
> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> + if (clkoutdiv != 1) {
> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> + return -EINVAL;
> + }
> +
> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {

When does that for loop terminate?

> + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> +
> + fvco = cfg->fbdiv * refclk;
> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> + pll_clk = fvco / VCU_PLL_DIV2;
> + if (fvco % VCU_PLL_DIV2 != 0)
> + pll_clk++;
> + mod = pll_clk % coreclk;
> + if (mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + } else if (coreclk - mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + divisor_core++;
> + } else {
> + continue;
> + }
> + if (divisor_core >= DIVISOR_MIN &&
> + divisor_core <= DIVISOR_MAX) {
> + found = cfg;
> + divisor_mcu = pll_clk / mcuclk;
> + mod = pll_clk % mcuclk;
> + if (mcuclk - mod < LIMIT)
> + divisor_mcu++;
> + break;
> + }
> + }
> + }
> +
> + if (!found) {
> + dev_err(xvcu->dev, "Invalid clock combination.\n");
> + return -EINVAL;
> + }

[snip]

--
~Randy

2017-12-17 06:08:10

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Randy,

Thanks a lot for the review.

> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Saturday, December 16, 2017 2:18 PM
> To: Dhaval Rajeshbhai Shah <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai
> Shah <[email protected]>
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
>
> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> > Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> > design created. This driver provides the processing system and
> > programmable logic isolation. Set the frequency based on the clock
> > information get from the logicoreIP register set.
> >
> > It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >
> > Signed-off-by: Dhaval Shah <[email protected]>
> > ---
> >
> > drivers/misc/Kconfig | 15 ++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/xlnx_vcu.c | 631
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 647 insertions(+)
> > create mode 100644 drivers/misc/xlnx_vcu.c
>
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 0000000..f489d34
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,631 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts Dhaval Shah <[email protected]>
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
>
> [snip]
>
>
> > +/**
> > + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> > + * @xvcu: Pointer to the xvcu_device structure
> > + *
> > + * Programming the VCU PLL based on the user configuration
> > + * (ref clock freq, core clock freq, mcu clock freq).
> > + * Core clock frequency has higher priority than mcu clock frequency
> > + * Errors in following cases
> > + * - When mcu or clock clock get from logicoreIP is 0
> > + * - When VCU PLL DIV related bits value other than 1
> > + * - When proper data not found for given data
> > + * - When sis570_1 clocksource related operation failed
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> > + u32 refclk, coreclk, mcuclk, inte, deci;
> > + u32 divisor_mcu, divisor_core, fvco;
> > + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> > + u32 cfg_val, mod, ctrl;
> > + int ret;
> > + unsigned int i;
> > + const struct xvcu_pll_cfg *found = NULL;
> > +
> > + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> > + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> > + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> > + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> > + if (!mcuclk || !coreclk) {
> > + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> > + return -EINVAL;
> > + }
> > +
> > + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> > + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> > + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> > + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> > +
> > + clk_disable_unprepare(xvcu->pll_ref);
> > + ret = clk_set_rate(xvcu->pll_ref, refclk);
> > + if (ret)
> > + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> > +
> > + ret = clk_prepare_enable(xvcu->pll_ref);
> > + if (ret) {
> > + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> > + return ret;
> > + }
> > +
> > + refclk = clk_get_rate(xvcu->pll_ref);
> > +
> > + /*
> > + * The divide-by-2 should be always enabled (==1)
> > + * to meet the timing in the design.
> > + * Otherwise, it's an error
> > + */
> > + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> > + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> > + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> > + if (clkoutdiv != 1) {
> > + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> > + return -EINVAL;
> > + }
> > +
> > + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
>
> When does that for loop terminate?
>
This loop will terminate either it reach to i =0 and one other case is when it find the proper expected value. Break statement is used to exit in that case.
> > + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> > +
> > + fvco = cfg->fbdiv * refclk;
> > + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> > + pll_clk = fvco / VCU_PLL_DIV2;
> > + if (fvco % VCU_PLL_DIV2 != 0)
> > + pll_clk++;
> > + mod = pll_clk % coreclk;
> > + if (mod < LIMIT) {
> > + divisor_core = pll_clk / coreclk;
> > + } else if (coreclk - mod < LIMIT) {
> > + divisor_core = pll_clk / coreclk;
> > + divisor_core++;
> > + } else {
> > + continue;
> > + }
> > + if (divisor_core >= DIVISOR_MIN &&
> > + divisor_core <= DIVISOR_MAX) {
> > + found = cfg;
> > + divisor_mcu = pll_clk / mcuclk;
> > + mod = pll_clk % mcuclk;
> > + if (mcuclk - mod < LIMIT)
> > + divisor_mcu++;
> > + break;
This is the break statement to exit when proper value is find.
> > + }
> > + }
> > + }
> > +
> > + if (!found) {
> > + dev_err(xvcu->dev, "Invalid clock combination.\n");
> > + return -EINVAL;
> > + }
>
> [snip]
>
> --
> ~Randy

2017-12-17 17:40:32

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> Hi Randy,
>
> Thanks a lot for the review.
>
>> -----Original Message-----
>> From: Randy Dunlap [mailto:[email protected]]
>> Sent: Saturday, December 16, 2017 2:18 PM
>> To: Dhaval Rajeshbhai Shah <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai
>> Shah <[email protected]>
>> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
>> init driver
>>
>> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
>>> design created. This driver provides the processing system and
>>> programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah <[email protected]>
>>> ---
>>>
>>> drivers/misc/Kconfig | 15 ++
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/xlnx_vcu.c | 631
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 647 insertions(+)
>>> create mode 100644 drivers/misc/xlnx_vcu.c
>>
>>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
>>> file mode 100644 index 0000000..f489d34
>>> --- /dev/null
>>> +++ b/drivers/misc/xlnx_vcu.c
>>> @@ -0,0 +1,631 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Xilinx VCU Init
>>> + *
>>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
>>> + *
>>> + * Contacts Dhaval Shah <[email protected]>
>>> + */
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>
>> [snip]
>>
>>
>>> +/**
>>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
>>> + * @xvcu: Pointer to the xvcu_device structure
>>> + *
>>> + * Programming the VCU PLL based on the user configuration
>>> + * (ref clock freq, core clock freq, mcu clock freq).
>>> + * Core clock frequency has higher priority than mcu clock frequency
>>> + * Errors in following cases
>>> + * - When mcu or clock clock get from logicoreIP is 0
>>> + * - When VCU PLL DIV related bits value other than 1
>>> + * - When proper data not found for given data
>>> + * - When sis570_1 clocksource related operation failed
>>> + *
>>> + * Return: Returns status, either success or error+reason
>>> + */
>>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
>>> + u32 refclk, coreclk, mcuclk, inte, deci;
>>> + u32 divisor_mcu, divisor_core, fvco;
>>> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
>>> + u32 cfg_val, mod, ctrl;
>>> + int ret;
>>> + unsigned int i;
>>> + const struct xvcu_pll_cfg *found = NULL;
>>> +
>>> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
>>> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
>>> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
>>> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
>>> + if (!mcuclk || !coreclk) {
>>> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
>>> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
>>> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
>>> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
>>> +
>>> + clk_disable_unprepare(xvcu->pll_ref);
>>> + ret = clk_set_rate(xvcu->pll_ref, refclk);
>>> + if (ret)
>>> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
>>> +
>>> + ret = clk_prepare_enable(xvcu->pll_ref);
>>> + if (ret) {
>>> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
>>> + return ret;
>>> + }
>>> +
>>> + refclk = clk_get_rate(xvcu->pll_ref);
>>> +
>>> + /*
>>> + * The divide-by-2 should be always enabled (==1)
>>> + * to meet the timing in the design.
>>> + * Otherwise, it's an error
>>> + */
>>> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
>>> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
>>> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
>>> + if (clkoutdiv != 1) {
>>> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
>>
>> When does that for loop terminate?
>>
> This loop will terminate either it reach to i =0 and one other case is when it find the proper expected value. Break statement is used to exit in that case.

Hm, well, I did mis-read it, but I think that there is still an off-by-one problem.

Consider the case of the value is not found in the array.

If ARRAY_SIZE(xvcu_pll_cfg) is 5:

for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {

i == 4:
check xvcu_pll_cfg[4]
i--;

i == 3:
check xvcu_pll_cfg[3]
i--;

i == 2;
check xvcu_pll_cfg[2]
i--;

i == 1:
check xvcu_pll_cfg[1]
i--;

i == 0;
terminate for-loop;
xvcu_pll_cfg[0] is never checked;

Is this clear?


>>> + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
>>> +
>>> + fvco = cfg->fbdiv * refclk;
>>> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
>>> + pll_clk = fvco / VCU_PLL_DIV2;
>>> + if (fvco % VCU_PLL_DIV2 != 0)
>>> + pll_clk++;
>>> + mod = pll_clk % coreclk;
>>> + if (mod < LIMIT) {
>>> + divisor_core = pll_clk / coreclk;
>>> + } else if (coreclk - mod < LIMIT) {
>>> + divisor_core = pll_clk / coreclk;
>>> + divisor_core++;
>>> + } else {
>>> + continue;
>>> + }
>>> + if (divisor_core >= DIVISOR_MIN &&
>>> + divisor_core <= DIVISOR_MAX) {
>>> + found = cfg;
>>> + divisor_mcu = pll_clk / mcuclk;
>>> + mod = pll_clk % mcuclk;
>>> + if (mcuclk - mod < LIMIT)
>>> + divisor_mcu++;
>>> + break;
> This is the break statement to exit when proper value is find.
>>> + }
>>> + }
>>> + }
>>> +
>>> + if (!found) {
>>> + dev_err(xvcu->dev, "Invalid clock combination.\n");
>>> + return -EINVAL;
>>> + }
>>
>> [snip]


--
~Randy

2017-12-18 05:38:22

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Randy,

> -----Original Message-----
> From: Randy Dunlap [mailto:[email protected]]
> Sent: Sunday, December 17, 2017 9:40 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Hyun Kwon <[email protected]>
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
>
> On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> > Hi Randy,
> >
> > Thanks a lot for the review.
> >
> >> -----Original Message-----
> >> From: Randy Dunlap [mailto:[email protected]]
> >> Sent: Saturday, December 16, 2017 2:18 PM
> >> To: Dhaval Rajeshbhai Shah <[email protected]>; [email protected];
> >> [email protected]; [email protected]; [email protected]
> >> Cc: [email protected]; [email protected];
> >> [email protected]; Hyun Kwon <[email protected]>; Dhaval
> >> Rajeshbhai Shah <[email protected]>
> >> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU
> >> logicoreIP init driver
> >>
> >> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> >>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> >>> design created. This driver provides the processing system and
> >>> programmable logic isolation. Set the frequency based on the clock
> >>> information get from the logicoreIP register set.
> >>>
> >>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >>>
> >>> Signed-off-by: Dhaval Shah <[email protected]>
> >>> ---
> >>>
> >>> drivers/misc/Kconfig | 15 ++
> >>> drivers/misc/Makefile | 1 +
> >>> drivers/misc/xlnx_vcu.c | 631
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++
> >>> 3 files changed, 647 insertions(+)
> >>> create mode 100644 drivers/misc/xlnx_vcu.c
> >>
> >>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> >>> file mode 100644 index 0000000..f489d34
> >>> --- /dev/null
> >>> +++ b/drivers/misc/xlnx_vcu.c
> >>> @@ -0,0 +1,631 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Xilinx VCU Init
> >>> + *
> >>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> >>> + *
> >>> + * Contacts Dhaval Shah <[email protected]>
> >>> + */
> >>> +#include <linux/clk.h>
> >>> +#include <linux/device.h>
> >>> +#include <linux/errno.h>
> >>> +#include <linux/io.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/of_platform.h>
> >>> +#include <linux/platform_device.h>
> >>
> >> [snip]
> >>
> >>
> >>> +/**
> >>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> >>> + * @xvcu: Pointer to the xvcu_device structure
> >>> + *
> >>> + * Programming the VCU PLL based on the user configuration
> >>> + * (ref clock freq, core clock freq, mcu clock freq).
> >>> + * Core clock frequency has higher priority than mcu clock
> >>> +frequency
> >>> + * Errors in following cases
> >>> + * - When mcu or clock clock get from logicoreIP is 0
> >>> + * - When VCU PLL DIV related bits value other than 1
> >>> + * - When proper data not found for given data
> >>> + * - When sis570_1 clocksource related operation failed
> >>> + *
> >>> + * Return: Returns status, either success or error+reason
> >>> + */
> >>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> >>> + u32 refclk, coreclk, mcuclk, inte, deci;
> >>> + u32 divisor_mcu, divisor_core, fvco;
> >>> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> >>> + u32 cfg_val, mod, ctrl;
> >>> + int ret;
> >>> + unsigned int i;
> >>> + const struct xvcu_pll_cfg *found = NULL;
> >>> +
> >>> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> >>> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> >>> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> >>> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> >>> + if (!mcuclk || !coreclk) {
> >>> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> >>> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> >>> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> >>> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> >>> +
> >>> + clk_disable_unprepare(xvcu->pll_ref);
> >>> + ret = clk_set_rate(xvcu->pll_ref, refclk);
> >>> + if (ret)
> >>> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> >>> +
> >>> + ret = clk_prepare_enable(xvcu->pll_ref);
> >>> + if (ret) {
> >>> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> >>> + return ret;
> >>> + }
> >>> +
> >>> + refclk = clk_get_rate(xvcu->pll_ref);
> >>> +
> >>> + /*
> >>> + * The divide-by-2 should be always enabled (==1)
> >>> + * to meet the timing in the design.
> >>> + * Otherwise, it's an error
> >>> + */
> >>> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> >>> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> >>> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> >>> + if (clkoutdiv != 1) {
> >>> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
> >>
> >> When does that for loop terminate?
> >>
> > This loop will terminate either it reach to i =0 and one other case is when it
> find the proper expected value. Break statement is used to exit in that case.
>
> Hm, well, I did mis-read it, but I think that there is still an off-by-one problem.
>
> Consider the case of the value is not found in the array.
>
> If ARRAY_SIZE(xvcu_pll_cfg) is 5:
>
> for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
>
> i == 4:
> check xvcu_pll_cfg[4]
> i--;
>
> i == 3:
> check xvcu_pll_cfg[3]
> i--;
>
> i == 2;
> check xvcu_pll_cfg[2]
> i--;
>
> i == 1:
> check xvcu_pll_cfg[1]
> i--;
>
> i == 0;
> terminate for-loop;
> xvcu_pll_cfg[0] is never checked;
>
> Is this clear?
>
Yes. You are right. It will miss index 0 in that case. I will update and send the next patch set.
>
> >>> + const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> >>> +
> >>> + fvco = cfg->fbdiv * refclk;
> >>> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> >>> + pll_clk = fvco / VCU_PLL_DIV2;
> >>> + if (fvco % VCU_PLL_DIV2 != 0)
> >>> + pll_clk++;
> >>> + mod = pll_clk % coreclk;
> >>> + if (mod < LIMIT) {
> >>> + divisor_core = pll_clk / coreclk;
> >>> + } else if (coreclk - mod < LIMIT) {
> >>> + divisor_core = pll_clk / coreclk;
> >>> + divisor_core++;
> >>> + } else {
> >>> + continue;
> >>> + }
> >>> + if (divisor_core >= DIVISOR_MIN &&
> >>> + divisor_core <= DIVISOR_MAX) {
> >>> + found = cfg;
> >>> + divisor_mcu = pll_clk / mcuclk;
> >>> + mod = pll_clk % mcuclk;
> >>> + if (mcuclk - mod < LIMIT)
> >>> + divisor_mcu++;
> >>> + break;
> > This is the break statement to exit when proper value is find.
> >>> + }
> >>> + }
> >>> + }
> >>> +
> >>> + if (!found) {
> >>> + dev_err(xvcu->dev, "Invalid clock combination.\n");
> >>> + return -EINVAL;
> >>> + }
> >>
> >> [snip]
>
>
> --
> ~Randy

2017-12-18 06:16:01

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v5 0/2] Documentation and driver of logicoreIP

1st patch provide Device Tree binding document for logicoreIP
2nd patch provide the xlnx_vcu logicoreIP driver, Kconfig changes
and Makefile changes for the driver.

Dhaval Shah (2):
Documentation: devicetree: Add DT bindings to xlnx_vcu driver
misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 +
drivers/misc/Kconfig | 15 +
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 630 +++++++++++++++++++++
4 files changed, 677 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt
create mode 100644 drivers/misc/xlnx_vcu.c

--
2.7.4

2017-12-18 06:16:12

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v5 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

Add Device Tree binding document for logicoreIP. This logicoreIP
provides the isolation between the processing system and
programmable logic. Also provides the clock related information.

Signed-off-by: Dhaval Shah <[email protected]>
---
Chnages since v5:
No Changes.
Chnages since v4:
No Changes.
Chnages since v3:
* Use "dt-bindings: misc: ..." for the subject.
Changes since v2:
* Describe the h/w
* compatible string is updated to make it more specific
based on the logicoreIP version.
* Removed that encoder and decoder child nodes and relatd properties as that
will be a separate driver and dts nodes. other team is working on that.
* Updated to use as a single driver.

.../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt

diff --git a/Documentation/devicetree/bindings/misc/xlnx,vcu.txt b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
new file mode 100644
index 0000000..6786d67
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,vcu.txt
@@ -0,0 +1,31 @@
+LogicoreIP designed compatible with Xilinx ZYNQ family.
+-------------------------------------------------------
+
+General concept
+---------------
+
+LogicoreIP design to provide the isolation between processing system
+and programmable logic. Also provides the list of register set to configure
+the frequency.
+
+Required properties:
+- compatible: shall be one of:
+ "xlnx,vcu"
+ "xlnx,vcu-logicoreip-1.0"
+- reg, reg-names: There are two sets of registers need to provide.
+ 1. vcu slcr
+ 2. Logicore
+ reg-names should contain name for the each register sequence.
+- clocks: phandle for aclk and pll_ref clocksource
+- clock-names: The identification string, "aclk", is always required for
+ the axi clock. "pll_ref" is required for pll.
+Example:
+
+ xlnx_vcu: vcu@a0040000 {
+ compatible = "xlnx,vcu-logicoreip-1.0";
+ reg = <0x0 0xa0040000 0x0 0x1000>,
+ <0x0 0xa0041000 0x0 0x1000>;
+ reg-names = "vcu_slcr", "logicore";
+ clocks = <&si570_1>, <&clkc 71>;
+ clock-names = "pll_ref", "aclk";
+ };
--
2.7.4

2017-12-18 06:16:23

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v5 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

Xilinx ZYNQMP logicoreIP Init driver is based on the new
LogiCoreIP design created. This driver provides the processing system
and programmable logic isolation. Set the frequency based on the clock
information get from the logicoreIP register set.

It is put in drivers/misc as there is no subsystem for this logicoreIP.

Signed-off-by: Dhaval Shah <[email protected]>
---
Changes since v5:
* Changes made to include index 0 of the array.
Changes since v4:
* Indent the help text (below) by 2 additional spaces, as documented in coding-style.rst
* Spell check are resolved as per the review comment.
* inline the read() and write() functions..
* multi-line comment style
* Updated subject line prefix
Changes since v3:
No Changes.
Changes since v2:
* Removed the "default n" from the Kconfig
* More help text added to explain more about the logicoreIP driver
* SPDX id is relocated at top of the file with // style comment
* Removed the export API and header file and make it a single driver
which provides logocoreIP init.
* Provide the information in commit message as well for the why driver
in drivers/misc.

drivers/misc/Kconfig | 15 ++
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 630 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 646 insertions(+)
create mode 100644 drivers/misc/xlnx_vcu.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f1a5c23..e679936 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
Enable this configuration option to enable the host side test driver
for PCI Endpoint.

+config XILINX_VCU
+ tristate "Xilinx VCU logicoreIP Init"
+ help
+ Provides the driver to enable and disable the isolation between the
+ processing system and programmable logic part by using the logicoreIP
+ register set. This driver also configures the frequency based on the
+ clock information from the logicoreIP register set.
+
+ If you say yes here you get support for the logicoreIP.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called xlnx_vcu.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 5ca5f64..a6bd0b1 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
+obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o

lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
new file mode 100644
index 0000000..46ec3fd
--- /dev/null
+++ b/drivers/misc/xlnx_vcu.c
@@ -0,0 +1,630 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+/* Address map for different registers implemented in the VCU LogiCORE IP. */
+#define VCU_ECODER_ENABLE 0x00
+#define VCU_DECODER_ENABLE 0x04
+#define VCU_MEMORY_DEPTH 0x08
+#define VCU_ENC_COLOR_DEPTH 0x0c
+#define VCU_ENC_VERTICAL_RANGE 0x10
+#define VCU_ENC_FRAME_SIZE_X 0x14
+#define VCU_ENC_FRAME_SIZE_Y 0x18
+#define VCU_ENC_COLOR_FORMAT 0x1c
+#define VCU_ENC_FPS 0x20
+#define VCU_MCU_CLK 0x24
+#define VCU_CORE_CLK 0x28
+#define VCU_PLL_BYPASS 0x2c
+#define VCU_ENC_CLK 0x30
+#define VCU_PLL_CLK 0x34
+#define VCU_ENC_VIDEO_STANDARD 0x38
+#define VCU_STATUS 0x3c
+#define VCU_AXI_ENC_CLK 0x40
+#define VCU_AXI_DEC_CLK 0x44
+#define VCU_AXI_MCU_CLK 0x48
+#define VCU_DEC_VIDEO_STANDARD 0x4c
+#define VCU_DEC_FRAME_SIZE_X 0x50
+#define VCU_DEC_FRAME_SIZE_Y 0x54
+#define VCU_DEC_FPS 0x58
+#define VCU_BUFFER_B_FRAME 0x5c
+#define VCU_WPP_EN 0x60
+#define VCU_PLL_CLK_DEC 0x64
+#define VCU_GASKET_INIT 0x74
+#define VCU_GASKET_VALUE 0x03
+
+/* vcu slcr registers, bitmask and shift */
+#define VCU_PLL_CTRL 0x24
+#define VCU_PLL_CTRL_RESET_MASK 0x01
+#define VCU_PLL_CTRL_RESET_SHIFT 0
+#define VCU_PLL_CTRL_BYPASS_MASK 0x01
+#define VCU_PLL_CTRL_BYPASS_SHIFT 3
+#define VCU_PLL_CTRL_FBDIV_MASK 0x7f
+#define VCU_PLL_CTRL_FBDIV_SHIFT 8
+#define VCU_PLL_CTRL_POR_IN_MASK 0x01
+#define VCU_PLL_CTRL_POR_IN_SHIFT 1
+#define VCU_PLL_CTRL_PWR_POR_MASK 0x01
+#define VCU_PLL_CTRL_PWR_POR_SHIFT 2
+#define VCU_PLL_CTRL_CLKOUTDIV_MASK 0x03
+#define VCU_PLL_CTRL_CLKOUTDIV_SHIFT 16
+#define VCU_PLL_CTRL_DEFAULT 0
+#define VCU_PLL_DIV2 2
+
+#define VCU_PLL_CFG 0x28
+#define VCU_PLL_CFG_RES_MASK 0x0f
+#define VCU_PLL_CFG_RES_SHIFT 0
+#define VCU_PLL_CFG_CP_MASK 0x0f
+#define VCU_PLL_CFG_CP_SHIFT 5
+#define VCU_PLL_CFG_LFHF_MASK 0x03
+#define VCU_PLL_CFG_LFHF_SHIFT 10
+#define VCU_PLL_CFG_LOCK_CNT_MASK 0x03ff
+#define VCU_PLL_CFG_LOCK_CNT_SHIFT 13
+#define VCU_PLL_CFG_LOCK_DLY_MASK 0x7f
+#define VCU_PLL_CFG_LOCK_DLY_SHIFT 25
+#define VCU_ENC_CORE_CTRL 0x30
+#define VCU_ENC_MCU_CTRL 0x34
+#define VCU_DEC_CORE_CTRL 0x38
+#define VCU_DEC_MCU_CTRL 0x3c
+#define VCU_PLL_DIVISOR_MASK 0x3f
+#define VCU_PLL_DIVISOR_SHIFT 4
+#define VCU_SRCSEL_MASK 0x01
+#define VCU_SRCSEL_SHIFT 0
+#define VCU_SRCSEL_PLL 1
+
+#define VCU_PLL_STATUS 0x60
+#define VCU_PLL_STATUS_LOCK_STATUS_MASK 0x01
+
+#define MHZ 1000000
+#define FVCO_MIN (1500U * MHZ)
+#define FVCO_MAX (3000U * MHZ)
+#define DIVISOR_MIN 0
+#define DIVISOR_MAX 63
+#define FRAC 100
+#define LIMIT (10 * MHZ)
+
+/**
+ * struct xvcu_device - Xilinx VCU init device structure
+ * @dev: Platform device
+ * @pll_ref: pll ref clock source
+ * @aclk: axi clock source
+ * @logicore_reg_ba: logicore reg base address
+ * @vcu_slcr_ba: vcu_slcr Register base address
+ * @coreclk: core clock frequency
+ */
+struct xvcu_device {
+ struct device *dev;
+ struct clk *pll_ref;
+ struct clk *aclk;
+ void __iomem *logicore_reg_ba;
+ void __iomem *vcu_slcr_ba;
+ u32 coreclk;
+};
+
+/**
+ * struct xvcu_pll_cfg - Helper data
+ * @fbdiv: The integer portion of the feedback divider to the PLL
+ * @cp: PLL charge pump control
+ * @res: PLL loop filter resistor control
+ * @lfhf: PLL loop filter high frequency capacitor control
+ * @lock_dly: Lock circuit configuration settings for lock windowsize
+ * @lock_cnt: Lock circuit counter setting
+ */
+struct xvcu_pll_cfg {
+ u32 fbdiv;
+ u32 cp;
+ u32 res;
+ u32 lfhf;
+ u32 lock_dly;
+ u32 lock_cnt;
+};
+
+static const struct xvcu_pll_cfg xvcu_pll_cfg[] = {
+ { 25, 3, 10, 3, 63, 1000 },
+ { 26, 3, 10, 3, 63, 1000 },
+ { 27, 4, 6, 3, 63, 1000 },
+ { 28, 4, 6, 3, 63, 1000 },
+ { 29, 4, 6, 3, 63, 1000 },
+ { 30, 4, 6, 3, 63, 1000 },
+ { 31, 6, 1, 3, 63, 1000 },
+ { 32, 6, 1, 3, 63, 1000 },
+ { 33, 4, 10, 3, 63, 1000 },
+ { 34, 5, 6, 3, 63, 1000 },
+ { 35, 5, 6, 3, 63, 1000 },
+ { 36, 5, 6, 3, 63, 1000 },
+ { 37, 5, 6, 3, 63, 1000 },
+ { 38, 5, 6, 3, 63, 975 },
+ { 39, 3, 12, 3, 63, 950 },
+ { 40, 3, 12, 3, 63, 925 },
+ { 41, 3, 12, 3, 63, 900 },
+ { 42, 3, 12, 3, 63, 875 },
+ { 43, 3, 12, 3, 63, 850 },
+ { 44, 3, 12, 3, 63, 850 },
+ { 45, 3, 12, 3, 63, 825 },
+ { 46, 3, 12, 3, 63, 800 },
+ { 47, 3, 12, 3, 63, 775 },
+ { 48, 3, 12, 3, 63, 775 },
+ { 49, 3, 12, 3, 63, 750 },
+ { 50, 3, 12, 3, 63, 750 },
+ { 51, 3, 2, 3, 63, 725 },
+ { 52, 3, 2, 3, 63, 700 },
+ { 53, 3, 2, 3, 63, 700 },
+ { 54, 3, 2, 3, 63, 675 },
+ { 55, 3, 2, 3, 63, 675 },
+ { 56, 3, 2, 3, 63, 650 },
+ { 57, 3, 2, 3, 63, 650 },
+ { 58, 3, 2, 3, 63, 625 },
+ { 59, 3, 2, 3, 63, 625 },
+ { 60, 3, 2, 3, 63, 625 },
+ { 61, 3, 2, 3, 63, 600 },
+ { 62, 3, 2, 3, 63, 600 },
+ { 63, 3, 2, 3, 63, 600 },
+ { 64, 3, 2, 3, 63, 600 },
+ { 65, 3, 2, 3, 63, 600 },
+ { 66, 3, 2, 3, 63, 600 },
+ { 67, 3, 2, 3, 63, 600 },
+ { 68, 3, 2, 3, 63, 600 },
+ { 69, 3, 2, 3, 63, 600 },
+ { 70, 3, 2, 3, 63, 600 },
+ { 71, 3, 2, 3, 63, 600 },
+ { 72, 3, 2, 3, 63, 600 },
+ { 73, 3, 2, 3, 63, 600 },
+ { 74, 3, 2, 3, 63, 600 },
+ { 75, 3, 2, 3, 63, 600 },
+ { 76, 3, 2, 3, 63, 600 },
+ { 77, 3, 2, 3, 63, 600 },
+ { 78, 3, 2, 3, 63, 600 },
+ { 79, 3, 2, 3, 63, 600 },
+ { 80, 3, 2, 3, 63, 600 },
+ { 81, 3, 2, 3, 63, 600 },
+ { 82, 3, 2, 3, 63, 600 },
+ { 83, 4, 2, 3, 63, 600 },
+ { 84, 4, 2, 3, 63, 600 },
+ { 85, 4, 2, 3, 63, 600 },
+ { 86, 4, 2, 3, 63, 600 },
+ { 87, 4, 2, 3, 63, 600 },
+ { 88, 4, 2, 3, 63, 600 },
+ { 89, 4, 2, 3, 63, 600 },
+ { 90, 4, 2, 3, 63, 600 },
+ { 91, 4, 2, 3, 63, 600 },
+ { 92, 4, 2, 3, 63, 600 },
+ { 93, 4, 2, 3, 63, 600 },
+ { 94, 4, 2, 3, 63, 600 },
+ { 95, 4, 2, 3, 63, 600 },
+ { 96, 4, 2, 3, 63, 600 },
+ { 97, 4, 2, 3, 63, 600 },
+ { 98, 4, 2, 3, 63, 600 },
+ { 99, 4, 2, 3, 63, 600 },
+ { 100, 4, 2, 3, 63, 600 },
+ { 101, 4, 2, 3, 63, 600 },
+ { 102, 4, 2, 3, 63, 600 },
+ { 103, 5, 2, 3, 63, 600 },
+ { 104, 5, 2, 3, 63, 600 },
+ { 105, 5, 2, 3, 63, 600 },
+ { 106, 5, 2, 3, 63, 600 },
+ { 107, 3, 4, 3, 63, 600 },
+ { 108, 3, 4, 3, 63, 600 },
+ { 109, 3, 4, 3, 63, 600 },
+ { 110, 3, 4, 3, 63, 600 },
+ { 111, 3, 4, 3, 63, 600 },
+ { 112, 3, 4, 3, 63, 600 },
+ { 113, 3, 4, 3, 63, 600 },
+ { 114, 3, 4, 3, 63, 600 },
+ { 115, 3, 4, 3, 63, 600 },
+ { 116, 3, 4, 3, 63, 600 },
+ { 117, 3, 4, 3, 63, 600 },
+ { 118, 3, 4, 3, 63, 600 },
+ { 119, 3, 4, 3, 63, 600 },
+ { 120, 3, 4, 3, 63, 600 },
+ { 121, 3, 4, 3, 63, 600 },
+ { 122, 3, 4, 3, 63, 600 },
+ { 123, 3, 4, 3, 63, 600 },
+ { 124, 3, 4, 3, 63, 600 },
+ { 125, 3, 4, 3, 63, 600 },
+};
+
+/**
+ * xvcu_read - Read from the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ *
+ * Return: Returns 32bit value from VCU register specified
+ *
+ */
+static inline u32 xvcu_read(void __iomem *iomem, u32 offset)
+{
+ return ioread32(iomem + offset);
+}
+
+/**
+ * xvcu_write - Write to the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @value: Value to write
+ */
+static inline void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
+{
+ iowrite32(value, iomem + offset);
+}
+
+/**
+ * xvcu_write_field_reg - Write to the vcu reg field
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @field: vcu reg field to write to
+ * @mask: vcu reg mask
+ * @shift: vcu reg number of bits to shift the bitfield
+ */
+static void xvcu_write_field_reg(void __iomem *iomem, int offset,
+ u32 field, u32 mask, int shift)
+{
+ u32 val = xvcu_read(iomem, offset);
+
+ val &= ~(mask << shift);
+ val |= (field & mask) << shift;
+
+ xvcu_write(iomem, offset, val);
+}
+
+/**
+ * xvcu_set_vcu_pll_info - Set the VCU PLL info
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Programming the VCU PLL based on the user configuration
+ * (ref clock freq, core clock freq, mcu clock freq).
+ * Core clock frequency has higher priority than mcu clock frequency
+ * Errors in following cases
+ * - When mcu or clock clock get from logicoreIP is 0
+ * - When VCU PLL DIV related bits value other than 1
+ * - When proper data not found for given data
+ * - When sis570_1 clocksource related operation failed
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
+{
+ u32 refclk, coreclk, mcuclk, inte, deci;
+ u32 divisor_mcu, divisor_core, fvco;
+ u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
+ u32 cfg_val, mod, ctrl;
+ int ret, i;
+ const struct xvcu_pll_cfg *found = NULL;
+
+ inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
+ deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
+ coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
+ mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
+ if (!mcuclk || !coreclk) {
+ dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
+ return -EINVAL;
+ }
+
+ refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
+ dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
+ dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ ret = clk_set_rate(xvcu->pll_ref, refclk);
+ if (ret)
+ dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
+ return ret;
+ }
+
+ refclk = clk_get_rate(xvcu->pll_ref);
+
+ /*
+ * The divide-by-2 should be always enabled (==1)
+ * to meet the timing in the design.
+ * Otherwise, it's an error
+ */
+ vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
+ clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
+ clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
+ if (clkoutdiv != 1) {
+ dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
+ return -EINVAL;
+ }
+
+ for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i >= 0; i--) {
+ const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
+
+ fvco = cfg->fbdiv * refclk;
+ if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
+ pll_clk = fvco / VCU_PLL_DIV2;
+ if (fvco % VCU_PLL_DIV2 != 0)
+ pll_clk++;
+ mod = pll_clk % coreclk;
+ if (mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ } else if (coreclk - mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ divisor_core++;
+ } else {
+ continue;
+ }
+ if (divisor_core >= DIVISOR_MIN &&
+ divisor_core <= DIVISOR_MAX) {
+ found = cfg;
+ divisor_mcu = pll_clk / mcuclk;
+ mod = pll_clk % mcuclk;
+ if (mcuclk - mod < LIMIT)
+ divisor_mcu++;
+ break;
+ }
+ }
+ }
+
+ if (!found) {
+ dev_err(xvcu->dev, "Invalid clock combination.\n");
+ return -EINVAL;
+ }
+
+ xvcu->coreclk = pll_clk / divisor_core;
+ mcuclk = pll_clk / divisor_mcu;
+ dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
+ dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
+
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
+ vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
+ VCU_PLL_CTRL_FBDIV_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
+ VCU_PLL_CTRL_POR_IN_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
+ VCU_PLL_CTRL_POR_IN_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+ /* Set divisor for the core and mcu clock */
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
+
+ /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
+ cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
+ (found->cp << VCU_PLL_CFG_CP_SHIFT) |
+ (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
+ (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
+ (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
+
+ return 0;
+}
+
+/**
+ * xvcu_set_pll - PLL init sequence
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Call the api to set the PLL info and once that is done then
+ * init the PLL sequence to make the PLL stable.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_pll(struct xvcu_device *xvcu)
+{
+ u32 lock_status;
+ unsigned long timeout;
+ int ret;
+
+ ret = xvcu_set_vcu_pll_info(xvcu);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to set pll info\n");
+ return ret;
+ }
+
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ /*
+ * Defined the timeout for the max time to wait the
+ * PLL_STATUS to be locked.
+ */
+ timeout = jiffies + msecs_to_jiffies(2000);
+ do {
+ lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
+ if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ return 0;
+ }
+ } while (!time_after(jiffies, timeout));
+
+ /* PLL is not locked even after the timeout of the 2sec */
+ dev_err(xvcu->dev, "PLL is not locked\n");
+ return -ETIMEDOUT;
+}
+
+/**
+ * xvcu_probe - Probe existence of the logicoreIP
+ * and initialize PLL
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct xvcu_device *xvcu;
+ int ret;
+
+ xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
+ if (!xvcu)
+ return -ENOMEM;
+
+ xvcu->dev = &pdev->dev;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
+ if (!res) {
+ dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->vcu_slcr_ba) {
+ dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
+ if (!res) {
+ dev_err(&pdev->dev, "get logicore memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->logicore_reg_ba) {
+ dev_err(&pdev->dev, "logicore register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
+ if (IS_ERR(xvcu->aclk)) {
+ dev_err(&pdev->dev, "Could not get aclk clock\n");
+ return PTR_ERR(xvcu->aclk);
+ }
+
+ xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
+ if (IS_ERR(xvcu->pll_ref)) {
+ dev_err(&pdev->dev, "Could not get pll_ref clock\n");
+ return PTR_ERR(xvcu->pll_ref);
+ }
+
+ ret = clk_prepare_enable(xvcu->aclk);
+ if (ret) {
+ dev_err(&pdev->dev, "aclk clock enable failed\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(&pdev->dev, "pll_ref clock enable failed\n");
+ goto error_aclk;
+ }
+
+ /*
+ * Do the Gasket isolation and put the VCU out of reset
+ * Bit 0 : Gasket isolation
+ * Bit 1 : put VCU out of reset
+ */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
+
+ /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
+ ret = xvcu_set_pll(xvcu);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to set the pll\n");
+ goto error_pll_ref;
+ }
+
+ dev_set_drvdata(&pdev->dev, xvcu);
+
+ dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
+
+ return 0;
+
+error_pll_ref:
+ clk_disable_unprepare(xvcu->pll_ref);
+error_aclk:
+ clk_disable_unprepare(xvcu->aclk);
+ return ret;
+}
+
+/**
+ * xvcu_remove - Insert gasket isolation
+ * and disable the clock
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_remove(struct platform_device *pdev)
+{
+ struct xvcu_device *xvcu;
+
+ xvcu = platform_get_drvdata(pdev);
+ if (!xvcu)
+ return -ENODEV;
+
+ /* Add the the Gasket isolation and put the VCU in reset. */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ clk_disable_unprepare(xvcu->aclk);
+
+ return 0;
+}
+
+static const struct of_device_id xvcu_of_id_table[] = {
+ { .compatible = "xlnx,vcu" },
+ { .compatible = "xlnx,vcu-logicoreip-1.0" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
+
+static struct platform_driver xvcu_driver = {
+ .driver = {
+ .name = "xilinx-vcu",
+ .of_match_table = xvcu_of_id_table,
+ },
+ .probe = xvcu_probe,
+ .remove = xvcu_remove,
+};
+
+module_platform_driver(xvcu_driver);
+
+MODULE_AUTHOR("Dhaval Shah <[email protected]>");
+MODULE_DESCRIPTION("Xilinx VCU init Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2017-12-18 13:13:55

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

On 15.12.2017 14:26, Arnd Bergmann wrote:
> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah <[email protected]> wrote:
>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>> LogiCoreIP design created. This driver provides the processing system
>> and programmable logic isolation. Set the frequency based on the clock
>> information get from the logicoreIP register set.
>>
>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>
>> Signed-off-by: Dhaval Shah <[email protected]>
>
> After giving this some more thought, I'd suggest you move the driver to
> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
> it merged by Michal Simek as a driver patch that will go through arm-soc.

I have not a problem of creating drivers/soc/xilinx/ location for this
driver. It is not zynq (arm32) but zynqmp(arm64) device where this
driver can be used. As far as I understand it is memory mapped soft IP
which could be also accessed by soft core CPU.
It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
We have been discussing that openrisc cases and for sure if someone
wants to enable this driver there using misc location would be one
option but I also think that using drivers/soc/xilinx location is not a
bad option because it is very unlikely that anybody tries it.

Arnd: misc or drivers/soc/xilinx?

Thanks,
Michal

2017-12-18 14:05:20

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek <[email protected]> wrote:
> On 15.12.2017 14:26, Arnd Bergmann wrote:
>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah <[email protected]> wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>>> LogiCoreIP design created. This driver provides the processing system
>>> and programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah <[email protected]>
>>
>> After giving this some more thought, I'd suggest you move the driver to
>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>
> I have not a problem of creating drivers/soc/xilinx/ location for this
> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
> driver can be used. As far as I understand it is memory mapped soft IP
> which could be also accessed by soft core CPU.

Ok. I wouldn't be worried about having a zynq directory for stuff that
is common between zynq and zynqmp, but the soft code CPU case
wouldn't make that ideal.

> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
> We have been discussing that openrisc cases and for sure if someone
> wants to enable this driver there using misc location would be one
> option but I also think that using drivers/soc/xilinx location is not a
> bad option because it is very unlikely that anybody tries it.
>
> Arnd: misc or drivers/soc/xilinx?

drivers/soc/xilinx please, thanks for the clarification.

Arnd

2017-12-19 13:25:17

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

On 18.12.2017 15:05, Arnd Bergmann wrote:
> On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek <[email protected]> wrote:
>> On 15.12.2017 14:26, Arnd Bergmann wrote:
>>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah <[email protected]> wrote:
>>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>>>> LogiCoreIP design created. This driver provides the processing system
>>>> and programmable logic isolation. Set the frequency based on the clock
>>>> information get from the logicoreIP register set.
>>>>
>>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>>
>>>> Signed-off-by: Dhaval Shah <[email protected]>
>>>
>>> After giving this some more thought, I'd suggest you move the driver to
>>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>>
>> I have not a problem of creating drivers/soc/xilinx/ location for this
>> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
>> driver can be used. As far as I understand it is memory mapped soft IP
>> which could be also accessed by soft core CPU.
>
> Ok. I wouldn't be worried about having a zynq directory for stuff that
> is common between zynq and zynqmp, but the soft code CPU case
> wouldn't make that ideal.
>
>> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
>> We have been discussing that openrisc cases and for sure if someone
>> wants to enable this driver there using misc location would be one
>> option but I also think that using drivers/soc/xilinx location is not a
>> bad option because it is very unlikely that anybody tries it.
>>
>> Arnd: misc or drivers/soc/xilinx?
>
> drivers/soc/xilinx please, thanks for the clarification.
>

ok. I have sent patch which prepare structures in drivers/soc/xilinx.

Dhaval: Please rebase your patch based on this and put driver to this
location.

Thanks,
Michal

2017-12-19 23:09:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

On Sun, Dec 17, 2017 at 10:15:31PM -0800, Dhaval Shah wrote:
> Add Device Tree binding document for logicoreIP. This logicoreIP
> provides the isolation between the processing system and
> programmable logic. Also provides the clock related information.
>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
> Chnages since v5:
> No Changes.
> Chnages since v4:
> No Changes.
> Chnages since v3:
> * Use "dt-bindings: misc: ..." for the subject.
> Changes since v2:
> * Describe the h/w
> * compatible string is updated to make it more specific
> based on the logicoreIP version.
> * Removed that encoder and decoder child nodes and relatd properties as that
> will be a separate driver and dts nodes. other team is working on that.
> * Updated to use as a single driver.
>
> .../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/xlnx,vcu.txt

Please add Reviewed-by tags when posting new versions.

Rob

2017-12-20 03:00:20

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH v5 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu driver

Hi Rob,

> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: Tuesday, December 19, 2017 3:10 PM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Hyun Kwon
> <[email protected]>; Dhaval Rajeshbhai Shah <[email protected]>
> Subject: Re: [PATCH v5 1/2] dt-bindings: misc: Add DT bindings to xlnx_vcu
> driver
>
> On Sun, Dec 17, 2017 at 10:15:31PM -0800, Dhaval Shah wrote:
> > Add Device Tree binding document for logicoreIP. This logicoreIP
> > provides the isolation between the processing system and programmable
> > logic. Also provides the clock related information.
> >
> > Signed-off-by: Dhaval Shah <[email protected]>
> > ---
> > Chnages since v5:
> > No Changes.
> > Chnages since v4:
> > No Changes.
> > Chnages since v3:
> > * Use "dt-bindings: misc: ..." for the subject.
> > Changes since v2:
> > * Describe the h/w
> > * compatible string is updated to make it more specific
> > based on the logicoreIP version.
> > * Removed that encoder and decoder child nodes and relatd properties as
> that
> > will be a separate driver and dts nodes. other team is working on that.
> > * Updated to use as a single driver.
> >
> > .../devicetree/bindings/misc/xlnx,vcu.txt | 31 ++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/misc/xlnx,vcu.txt
>
> Please add Reviewed-by tags when posting new versions.
>
> Rob

Sure. I will add this tag in the next version of patch.

Thanks & Regards,
Dhaval

2017-12-21 18:34:04

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v6 2/2] soc: xilinx: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

Xilinx ZYNQMP logicoreIP Init driver is based on the new
LogiCoreIP design created. This driver provides the processing system
and programmable logic isolation. Set the frequency based on the clock
information get from the logicoreIP register set.

Signed-off-by: Dhaval Shah <[email protected]>
---
Changes since v6:
* Path of the driver is updated to "drivers/soc/xilinx/" from "drivers/misc"
* Patch is rebase on top of the https://patchwork.kernel.org/patch/10123235/
Changes since v5:
* Changes made to include index 0 of the array.
Changes since v4:
* Indent the help text (below) by 2 additional spaces, as documented in coding-style.rst
* Spell check are resolved as per the review comment.
* inline the read() and write() functions..
* multi-line comment style
* Updated subject line prefix
Changes since v3:
No Changes.
Changes since v2:
* Removed the "default n" from the Kconfig
* More help text added to explain more about the logicoreIP driver
* SPDX id is relocated at top of the file with // style comment
* Removed the export API and header file and make it a single driver
which provides logocoreIP init.
* Provide the information in commit message as well for the why driver
in drivers/misc.

drivers/soc/xilinx/Kconfig | 15 +
drivers/soc/xilinx/Makefile | 1 +
drivers/soc/xilinx/xlnx_vcu.c | 630 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 646 insertions(+)
create mode 100644 drivers/soc/xilinx/xlnx_vcu.c

diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index ffaaa2f..266b50f 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -1,4 +1,19 @@
# SPDX-License-Identifier: GPL-2.0
menu "Xilinx SoC drivers"

+config XILINX_VCU
+ tristate "Xilinx VCU logicoreIP Init"
+ help
+ Provides the driver to enable and disable the isolation between the
+ processing system and programmable logic part by using the logicoreIP
+ register set. This driver also configures the frequency based on the
+ clock information from the logicoreIP register set.
+
+ If you say yes here you get support for the logicoreIP.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called xlnx_vcu.
+
endmenu
diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
index f66554c..dee8fd5 100644
--- a/drivers/soc/xilinx/Makefile
+++ b/drivers/soc/xilinx/Makefile
@@ -1 +1,2 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_XILINX_VCU) += xlnx_vcu.o
diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
new file mode 100644
index 0000000..46ec3fd
--- /dev/null
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -0,0 +1,630 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+/* Address map for different registers implemented in the VCU LogiCORE IP. */
+#define VCU_ECODER_ENABLE 0x00
+#define VCU_DECODER_ENABLE 0x04
+#define VCU_MEMORY_DEPTH 0x08
+#define VCU_ENC_COLOR_DEPTH 0x0c
+#define VCU_ENC_VERTICAL_RANGE 0x10
+#define VCU_ENC_FRAME_SIZE_X 0x14
+#define VCU_ENC_FRAME_SIZE_Y 0x18
+#define VCU_ENC_COLOR_FORMAT 0x1c
+#define VCU_ENC_FPS 0x20
+#define VCU_MCU_CLK 0x24
+#define VCU_CORE_CLK 0x28
+#define VCU_PLL_BYPASS 0x2c
+#define VCU_ENC_CLK 0x30
+#define VCU_PLL_CLK 0x34
+#define VCU_ENC_VIDEO_STANDARD 0x38
+#define VCU_STATUS 0x3c
+#define VCU_AXI_ENC_CLK 0x40
+#define VCU_AXI_DEC_CLK 0x44
+#define VCU_AXI_MCU_CLK 0x48
+#define VCU_DEC_VIDEO_STANDARD 0x4c
+#define VCU_DEC_FRAME_SIZE_X 0x50
+#define VCU_DEC_FRAME_SIZE_Y 0x54
+#define VCU_DEC_FPS 0x58
+#define VCU_BUFFER_B_FRAME 0x5c
+#define VCU_WPP_EN 0x60
+#define VCU_PLL_CLK_DEC 0x64
+#define VCU_GASKET_INIT 0x74
+#define VCU_GASKET_VALUE 0x03
+
+/* vcu slcr registers, bitmask and shift */
+#define VCU_PLL_CTRL 0x24
+#define VCU_PLL_CTRL_RESET_MASK 0x01
+#define VCU_PLL_CTRL_RESET_SHIFT 0
+#define VCU_PLL_CTRL_BYPASS_MASK 0x01
+#define VCU_PLL_CTRL_BYPASS_SHIFT 3
+#define VCU_PLL_CTRL_FBDIV_MASK 0x7f
+#define VCU_PLL_CTRL_FBDIV_SHIFT 8
+#define VCU_PLL_CTRL_POR_IN_MASK 0x01
+#define VCU_PLL_CTRL_POR_IN_SHIFT 1
+#define VCU_PLL_CTRL_PWR_POR_MASK 0x01
+#define VCU_PLL_CTRL_PWR_POR_SHIFT 2
+#define VCU_PLL_CTRL_CLKOUTDIV_MASK 0x03
+#define VCU_PLL_CTRL_CLKOUTDIV_SHIFT 16
+#define VCU_PLL_CTRL_DEFAULT 0
+#define VCU_PLL_DIV2 2
+
+#define VCU_PLL_CFG 0x28
+#define VCU_PLL_CFG_RES_MASK 0x0f
+#define VCU_PLL_CFG_RES_SHIFT 0
+#define VCU_PLL_CFG_CP_MASK 0x0f
+#define VCU_PLL_CFG_CP_SHIFT 5
+#define VCU_PLL_CFG_LFHF_MASK 0x03
+#define VCU_PLL_CFG_LFHF_SHIFT 10
+#define VCU_PLL_CFG_LOCK_CNT_MASK 0x03ff
+#define VCU_PLL_CFG_LOCK_CNT_SHIFT 13
+#define VCU_PLL_CFG_LOCK_DLY_MASK 0x7f
+#define VCU_PLL_CFG_LOCK_DLY_SHIFT 25
+#define VCU_ENC_CORE_CTRL 0x30
+#define VCU_ENC_MCU_CTRL 0x34
+#define VCU_DEC_CORE_CTRL 0x38
+#define VCU_DEC_MCU_CTRL 0x3c
+#define VCU_PLL_DIVISOR_MASK 0x3f
+#define VCU_PLL_DIVISOR_SHIFT 4
+#define VCU_SRCSEL_MASK 0x01
+#define VCU_SRCSEL_SHIFT 0
+#define VCU_SRCSEL_PLL 1
+
+#define VCU_PLL_STATUS 0x60
+#define VCU_PLL_STATUS_LOCK_STATUS_MASK 0x01
+
+#define MHZ 1000000
+#define FVCO_MIN (1500U * MHZ)
+#define FVCO_MAX (3000U * MHZ)
+#define DIVISOR_MIN 0
+#define DIVISOR_MAX 63
+#define FRAC 100
+#define LIMIT (10 * MHZ)
+
+/**
+ * struct xvcu_device - Xilinx VCU init device structure
+ * @dev: Platform device
+ * @pll_ref: pll ref clock source
+ * @aclk: axi clock source
+ * @logicore_reg_ba: logicore reg base address
+ * @vcu_slcr_ba: vcu_slcr Register base address
+ * @coreclk: core clock frequency
+ */
+struct xvcu_device {
+ struct device *dev;
+ struct clk *pll_ref;
+ struct clk *aclk;
+ void __iomem *logicore_reg_ba;
+ void __iomem *vcu_slcr_ba;
+ u32 coreclk;
+};
+
+/**
+ * struct xvcu_pll_cfg - Helper data
+ * @fbdiv: The integer portion of the feedback divider to the PLL
+ * @cp: PLL charge pump control
+ * @res: PLL loop filter resistor control
+ * @lfhf: PLL loop filter high frequency capacitor control
+ * @lock_dly: Lock circuit configuration settings for lock windowsize
+ * @lock_cnt: Lock circuit counter setting
+ */
+struct xvcu_pll_cfg {
+ u32 fbdiv;
+ u32 cp;
+ u32 res;
+ u32 lfhf;
+ u32 lock_dly;
+ u32 lock_cnt;
+};
+
+static const struct xvcu_pll_cfg xvcu_pll_cfg[] = {
+ { 25, 3, 10, 3, 63, 1000 },
+ { 26, 3, 10, 3, 63, 1000 },
+ { 27, 4, 6, 3, 63, 1000 },
+ { 28, 4, 6, 3, 63, 1000 },
+ { 29, 4, 6, 3, 63, 1000 },
+ { 30, 4, 6, 3, 63, 1000 },
+ { 31, 6, 1, 3, 63, 1000 },
+ { 32, 6, 1, 3, 63, 1000 },
+ { 33, 4, 10, 3, 63, 1000 },
+ { 34, 5, 6, 3, 63, 1000 },
+ { 35, 5, 6, 3, 63, 1000 },
+ { 36, 5, 6, 3, 63, 1000 },
+ { 37, 5, 6, 3, 63, 1000 },
+ { 38, 5, 6, 3, 63, 975 },
+ { 39, 3, 12, 3, 63, 950 },
+ { 40, 3, 12, 3, 63, 925 },
+ { 41, 3, 12, 3, 63, 900 },
+ { 42, 3, 12, 3, 63, 875 },
+ { 43, 3, 12, 3, 63, 850 },
+ { 44, 3, 12, 3, 63, 850 },
+ { 45, 3, 12, 3, 63, 825 },
+ { 46, 3, 12, 3, 63, 800 },
+ { 47, 3, 12, 3, 63, 775 },
+ { 48, 3, 12, 3, 63, 775 },
+ { 49, 3, 12, 3, 63, 750 },
+ { 50, 3, 12, 3, 63, 750 },
+ { 51, 3, 2, 3, 63, 725 },
+ { 52, 3, 2, 3, 63, 700 },
+ { 53, 3, 2, 3, 63, 700 },
+ { 54, 3, 2, 3, 63, 675 },
+ { 55, 3, 2, 3, 63, 675 },
+ { 56, 3, 2, 3, 63, 650 },
+ { 57, 3, 2, 3, 63, 650 },
+ { 58, 3, 2, 3, 63, 625 },
+ { 59, 3, 2, 3, 63, 625 },
+ { 60, 3, 2, 3, 63, 625 },
+ { 61, 3, 2, 3, 63, 600 },
+ { 62, 3, 2, 3, 63, 600 },
+ { 63, 3, 2, 3, 63, 600 },
+ { 64, 3, 2, 3, 63, 600 },
+ { 65, 3, 2, 3, 63, 600 },
+ { 66, 3, 2, 3, 63, 600 },
+ { 67, 3, 2, 3, 63, 600 },
+ { 68, 3, 2, 3, 63, 600 },
+ { 69, 3, 2, 3, 63, 600 },
+ { 70, 3, 2, 3, 63, 600 },
+ { 71, 3, 2, 3, 63, 600 },
+ { 72, 3, 2, 3, 63, 600 },
+ { 73, 3, 2, 3, 63, 600 },
+ { 74, 3, 2, 3, 63, 600 },
+ { 75, 3, 2, 3, 63, 600 },
+ { 76, 3, 2, 3, 63, 600 },
+ { 77, 3, 2, 3, 63, 600 },
+ { 78, 3, 2, 3, 63, 600 },
+ { 79, 3, 2, 3, 63, 600 },
+ { 80, 3, 2, 3, 63, 600 },
+ { 81, 3, 2, 3, 63, 600 },
+ { 82, 3, 2, 3, 63, 600 },
+ { 83, 4, 2, 3, 63, 600 },
+ { 84, 4, 2, 3, 63, 600 },
+ { 85, 4, 2, 3, 63, 600 },
+ { 86, 4, 2, 3, 63, 600 },
+ { 87, 4, 2, 3, 63, 600 },
+ { 88, 4, 2, 3, 63, 600 },
+ { 89, 4, 2, 3, 63, 600 },
+ { 90, 4, 2, 3, 63, 600 },
+ { 91, 4, 2, 3, 63, 600 },
+ { 92, 4, 2, 3, 63, 600 },
+ { 93, 4, 2, 3, 63, 600 },
+ { 94, 4, 2, 3, 63, 600 },
+ { 95, 4, 2, 3, 63, 600 },
+ { 96, 4, 2, 3, 63, 600 },
+ { 97, 4, 2, 3, 63, 600 },
+ { 98, 4, 2, 3, 63, 600 },
+ { 99, 4, 2, 3, 63, 600 },
+ { 100, 4, 2, 3, 63, 600 },
+ { 101, 4, 2, 3, 63, 600 },
+ { 102, 4, 2, 3, 63, 600 },
+ { 103, 5, 2, 3, 63, 600 },
+ { 104, 5, 2, 3, 63, 600 },
+ { 105, 5, 2, 3, 63, 600 },
+ { 106, 5, 2, 3, 63, 600 },
+ { 107, 3, 4, 3, 63, 600 },
+ { 108, 3, 4, 3, 63, 600 },
+ { 109, 3, 4, 3, 63, 600 },
+ { 110, 3, 4, 3, 63, 600 },
+ { 111, 3, 4, 3, 63, 600 },
+ { 112, 3, 4, 3, 63, 600 },
+ { 113, 3, 4, 3, 63, 600 },
+ { 114, 3, 4, 3, 63, 600 },
+ { 115, 3, 4, 3, 63, 600 },
+ { 116, 3, 4, 3, 63, 600 },
+ { 117, 3, 4, 3, 63, 600 },
+ { 118, 3, 4, 3, 63, 600 },
+ { 119, 3, 4, 3, 63, 600 },
+ { 120, 3, 4, 3, 63, 600 },
+ { 121, 3, 4, 3, 63, 600 },
+ { 122, 3, 4, 3, 63, 600 },
+ { 123, 3, 4, 3, 63, 600 },
+ { 124, 3, 4, 3, 63, 600 },
+ { 125, 3, 4, 3, 63, 600 },
+};
+
+/**
+ * xvcu_read - Read from the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ *
+ * Return: Returns 32bit value from VCU register specified
+ *
+ */
+static inline u32 xvcu_read(void __iomem *iomem, u32 offset)
+{
+ return ioread32(iomem + offset);
+}
+
+/**
+ * xvcu_write - Write to the VCU register space
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @value: Value to write
+ */
+static inline void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
+{
+ iowrite32(value, iomem + offset);
+}
+
+/**
+ * xvcu_write_field_reg - Write to the vcu reg field
+ * @iomem: vcu reg space base address
+ * @offset: vcu reg offset from base
+ * @field: vcu reg field to write to
+ * @mask: vcu reg mask
+ * @shift: vcu reg number of bits to shift the bitfield
+ */
+static void xvcu_write_field_reg(void __iomem *iomem, int offset,
+ u32 field, u32 mask, int shift)
+{
+ u32 val = xvcu_read(iomem, offset);
+
+ val &= ~(mask << shift);
+ val |= (field & mask) << shift;
+
+ xvcu_write(iomem, offset, val);
+}
+
+/**
+ * xvcu_set_vcu_pll_info - Set the VCU PLL info
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Programming the VCU PLL based on the user configuration
+ * (ref clock freq, core clock freq, mcu clock freq).
+ * Core clock frequency has higher priority than mcu clock frequency
+ * Errors in following cases
+ * - When mcu or clock clock get from logicoreIP is 0
+ * - When VCU PLL DIV related bits value other than 1
+ * - When proper data not found for given data
+ * - When sis570_1 clocksource related operation failed
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
+{
+ u32 refclk, coreclk, mcuclk, inte, deci;
+ u32 divisor_mcu, divisor_core, fvco;
+ u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
+ u32 cfg_val, mod, ctrl;
+ int ret, i;
+ const struct xvcu_pll_cfg *found = NULL;
+
+ inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
+ deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
+ coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
+ mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
+ if (!mcuclk || !coreclk) {
+ dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
+ return -EINVAL;
+ }
+
+ refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
+ dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
+ dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ ret = clk_set_rate(xvcu->pll_ref, refclk);
+ if (ret)
+ dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
+ return ret;
+ }
+
+ refclk = clk_get_rate(xvcu->pll_ref);
+
+ /*
+ * The divide-by-2 should be always enabled (==1)
+ * to meet the timing in the design.
+ * Otherwise, it's an error
+ */
+ vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
+ clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
+ clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
+ if (clkoutdiv != 1) {
+ dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
+ return -EINVAL;
+ }
+
+ for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i >= 0; i--) {
+ const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
+
+ fvco = cfg->fbdiv * refclk;
+ if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
+ pll_clk = fvco / VCU_PLL_DIV2;
+ if (fvco % VCU_PLL_DIV2 != 0)
+ pll_clk++;
+ mod = pll_clk % coreclk;
+ if (mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ } else if (coreclk - mod < LIMIT) {
+ divisor_core = pll_clk / coreclk;
+ divisor_core++;
+ } else {
+ continue;
+ }
+ if (divisor_core >= DIVISOR_MIN &&
+ divisor_core <= DIVISOR_MAX) {
+ found = cfg;
+ divisor_mcu = pll_clk / mcuclk;
+ mod = pll_clk % mcuclk;
+ if (mcuclk - mod < LIMIT)
+ divisor_mcu++;
+ break;
+ }
+ }
+ }
+
+ if (!found) {
+ dev_err(xvcu->dev, "Invalid clock combination.\n");
+ return -EINVAL;
+ }
+
+ xvcu->coreclk = pll_clk / divisor_core;
+ mcuclk = pll_clk / divisor_mcu;
+ dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
+ dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
+ dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
+
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
+ vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
+ VCU_PLL_CTRL_FBDIV_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
+ VCU_PLL_CTRL_POR_IN_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
+ VCU_PLL_CTRL_POR_IN_SHIFT;
+ vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT);
+ vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
+ VCU_PLL_CTRL_PWR_POR_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
+
+ /* Set divisor for the core and mcu clock */
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
+ VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
+
+ ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
+ ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
+ ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
+ ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
+ ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
+
+ /* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
+ cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
+ (found->cp << VCU_PLL_CFG_CP_SHIFT) |
+ (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
+ (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
+ (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
+ xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
+
+ return 0;
+}
+
+/**
+ * xvcu_set_pll - PLL init sequence
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Call the api to set the PLL info and once that is done then
+ * init the PLL sequence to make the PLL stable.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int xvcu_set_pll(struct xvcu_device *xvcu)
+{
+ u32 lock_status;
+ unsigned long timeout;
+ int ret;
+
+ ret = xvcu_set_vcu_pll_info(xvcu);
+ if (ret) {
+ dev_err(xvcu->dev, "failed to set pll info\n");
+ return ret;
+ }
+
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 1, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_RESET_MASK,
+ VCU_PLL_CTRL_RESET_SHIFT);
+ /*
+ * Defined the timeout for the max time to wait the
+ * PLL_STATUS to be locked.
+ */
+ timeout = jiffies + msecs_to_jiffies(2000);
+ do {
+ lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
+ if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
+ xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
+ 0, VCU_PLL_CTRL_BYPASS_MASK,
+ VCU_PLL_CTRL_BYPASS_SHIFT);
+ return 0;
+ }
+ } while (!time_after(jiffies, timeout));
+
+ /* PLL is not locked even after the timeout of the 2sec */
+ dev_err(xvcu->dev, "PLL is not locked\n");
+ return -ETIMEDOUT;
+}
+
+/**
+ * xvcu_probe - Probe existence of the logicoreIP
+ * and initialize PLL
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct xvcu_device *xvcu;
+ int ret;
+
+ xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
+ if (!xvcu)
+ return -ENOMEM;
+
+ xvcu->dev = &pdev->dev;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
+ if (!res) {
+ dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->vcu_slcr_ba) {
+ dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
+ if (!res) {
+ dev_err(&pdev->dev, "get logicore memory resource failed.\n");
+ return -ENODEV;
+ }
+
+ xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
+ res->start, resource_size(res));
+ if (!xvcu->logicore_reg_ba) {
+ dev_err(&pdev->dev, "logicore register mapping failed.\n");
+ return -ENOMEM;
+ }
+
+ xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
+ if (IS_ERR(xvcu->aclk)) {
+ dev_err(&pdev->dev, "Could not get aclk clock\n");
+ return PTR_ERR(xvcu->aclk);
+ }
+
+ xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
+ if (IS_ERR(xvcu->pll_ref)) {
+ dev_err(&pdev->dev, "Could not get pll_ref clock\n");
+ return PTR_ERR(xvcu->pll_ref);
+ }
+
+ ret = clk_prepare_enable(xvcu->aclk);
+ if (ret) {
+ dev_err(&pdev->dev, "aclk clock enable failed\n");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(xvcu->pll_ref);
+ if (ret) {
+ dev_err(&pdev->dev, "pll_ref clock enable failed\n");
+ goto error_aclk;
+ }
+
+ /*
+ * Do the Gasket isolation and put the VCU out of reset
+ * Bit 0 : Gasket isolation
+ * Bit 1 : put VCU out of reset
+ */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
+
+ /* Do the PLL Settings based on the ref clk,core and mcu clk freq */
+ ret = xvcu_set_pll(xvcu);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to set the pll\n");
+ goto error_pll_ref;
+ }
+
+ dev_set_drvdata(&pdev->dev, xvcu);
+
+ dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
+
+ return 0;
+
+error_pll_ref:
+ clk_disable_unprepare(xvcu->pll_ref);
+error_aclk:
+ clk_disable_unprepare(xvcu->aclk);
+ return ret;
+}
+
+/**
+ * xvcu_remove - Insert gasket isolation
+ * and disable the clock
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Returns 0 on success
+ * Negative error code otherwise
+ */
+static int xvcu_remove(struct platform_device *pdev)
+{
+ struct xvcu_device *xvcu;
+
+ xvcu = platform_get_drvdata(pdev);
+ if (!xvcu)
+ return -ENODEV;
+
+ /* Add the the Gasket isolation and put the VCU in reset. */
+ xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
+
+ clk_disable_unprepare(xvcu->pll_ref);
+ clk_disable_unprepare(xvcu->aclk);
+
+ return 0;
+}
+
+static const struct of_device_id xvcu_of_id_table[] = {
+ { .compatible = "xlnx,vcu" },
+ { .compatible = "xlnx,vcu-logicoreip-1.0" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
+
+static struct platform_driver xvcu_driver = {
+ .driver = {
+ .name = "xilinx-vcu",
+ .of_match_table = xvcu_of_id_table,
+ },
+ .probe = xvcu_probe,
+ .remove = xvcu_remove,
+};
+
+module_platform_driver(xvcu_driver);
+
+MODULE_AUTHOR("Dhaval Shah <[email protected]>");
+MODULE_DESCRIPTION("Xilinx VCU init Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2017-12-21 18:34:26

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v6 1/2] dt-bindings: soc: xilinx: Add DT bindings to xlnx_vcu driver

Add Device Tree binding document for logicoreIP. This logicoreIP
provides the isolation between the processing system and
programmable logic. Also provides the clock related information.

Signed-off-by: Dhaval Shah <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes since v6:
* Updated path of the dt-bindings doc as driver path is updated.
Chnages since v5:
No Changes.
Chnages since v4:
No Changes.
Chnages since v3:
* Use "dt-bindings: misc: ..." for the subject.
Changes since v2:
* Describe the h/w
* compatible string is updated to make it more specific
based on the logicoreIP version.
* Removed that encoder and decoder child nodes and relatd properties as that
will be a separate driver and dts nodes. other team is working on that.
* Updated to use as a single driver.

.../devicetree/bindings/soc/xilinx/xlnx,vcu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt

diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
new file mode 100644
index 0000000..6786d67
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
@@ -0,0 +1,31 @@
+LogicoreIP designed compatible with Xilinx ZYNQ family.
+-------------------------------------------------------
+
+General concept
+---------------
+
+LogicoreIP design to provide the isolation between processing system
+and programmable logic. Also provides the list of register set to configure
+the frequency.
+
+Required properties:
+- compatible: shall be one of:
+ "xlnx,vcu"
+ "xlnx,vcu-logicoreip-1.0"
+- reg, reg-names: There are two sets of registers need to provide.
+ 1. vcu slcr
+ 2. Logicore
+ reg-names should contain name for the each register sequence.
+- clocks: phandle for aclk and pll_ref clocksource
+- clock-names: The identification string, "aclk", is always required for
+ the axi clock. "pll_ref" is required for pll.
+Example:
+
+ xlnx_vcu: vcu@a0040000 {
+ compatible = "xlnx,vcu-logicoreip-1.0";
+ reg = <0x0 0xa0040000 0x0 0x1000>,
+ <0x0 0xa0041000 0x0 0x1000>;
+ reg-names = "vcu_slcr", "logicore";
+ clocks = <&si570_1>, <&clkc 71>;
+ clock-names = "pll_ref", "aclk";
+ };
--
2.7.4

2017-12-21 18:34:53

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v6 0/2] Documentation and driver of logicoreIP

1st patch provide Device Tree binding document for logicoreIP
2nd patch provide the xlnx_vcu logicoreIP driver, Kconfig changes
and Makefile changes for the driver.

Dhaval Shah (2):
dt-bindings: misc: Add DT bindings to xlnx_vcu driver
misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

.../devicetree/bindings/soc/xilinx/xlnx,vcu.txt | 31 +
drivers/soc/xilinx/Kconfig | 15 +
drivers/soc/xilinx/Makefile | 1 +
drivers/soc/xilinx/xlnx_vcu.c | 630 +++++++++++++++++++++
4 files changed, 677 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
create mode 100644 drivers/soc/xilinx/xlnx_vcu.c

--
2.7.4

2018-01-08 12:43:52

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v6 0/2] Documentation and driver of logicoreIP

On 21.12.2017 19:33, Dhaval Shah wrote:
> 1st patch provide Device Tree binding document for logicoreIP
> 2nd patch provide the xlnx_vcu logicoreIP driver, Kconfig changes
> and Makefile changes for the driver.
>
> Dhaval Shah (2):
> dt-bindings: misc: Add DT bindings to xlnx_vcu driver
> misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver
>
> .../devicetree/bindings/soc/xilinx/xlnx,vcu.txt | 31 +
> drivers/soc/xilinx/Kconfig | 15 +
> drivers/soc/xilinx/Makefile | 1 +
> drivers/soc/xilinx/xlnx_vcu.c | 630 +++++++++++++++++++++
> 4 files changed, 677 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
> create mode 100644 drivers/soc/xilinx/xlnx_vcu.c
>

It looks like there is no any other concern regarding these patches
that's why I have applied them.

Thanks,
Michal