2017-12-05 11:55:29

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
created. This driver will provide the api which can be used
by the encoder and decoder driver to get the configured value.

Signed-off-by: Dhaval Shah <[email protected]>
---
drivers/misc/Kconfig | 6 +
drivers/misc/Makefile | 1 +
drivers/misc/xlnx_vcu.c | 664 ++++++++++++++++++++++++++++++++++++++++++++++++
include/misc/xlnx_vcu.h | 18 ++
4 files changed, 689 insertions(+)
create mode 100644 drivers/misc/xlnx_vcu.c
create mode 100644 include/misc/xlnx_vcu.h

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

+config XILINX_VCU
+ tristate "Xilinx VCU Init"
+ default n
+ help
+ Driver for the Xilinx VCU Init based on the logicoreIP.
+
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..373f7f9
--- /dev/null
+++ b/drivers/misc/xlnx_vcu.c
@@ -0,0 +1,664 @@
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+#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>
+
+#include <misc/xlnx_vcu.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_get_color_depth - read the color depth register
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Return: Returns 32bit value
+ *
+ */
+u32 xvcu_get_color_depth(struct xvcu_device *xvcu)
+{
+ return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH);
+}
+EXPORT_SYMBOL_GPL(xvcu_get_color_depth);
+
+/**
+ * xvcu_get_memory_depth - read the memory depth register
+ * @xvcu: Pointer to the xvcu_device structure
+ *
+ * Return: Returns 32bit value
+ *
+ */
+u32 xvcu_get_memory_depth(struct xvcu_device *xvcu)
+{
+ return xvcu_read(xvcu->logicore_reg_ba, VCU_MEMORY_DEPTH);
+}
+EXPORT_SYMBOL_GPL(xvcu_get_memory_depth);
+
+/**
+ * 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);
+
+ ret = of_platform_populate(xvcu->dev->of_node, NULL, NULL, &pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register codecs\n");
+ goto error_pll_ref;
+ }
+ 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 - Depopulate the child nodes, 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;
+
+ of_platform_depopulate(&pdev->dev);
+
+ /* 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" },
+ { }
+};
+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");
diff --git a/include/misc/xlnx_vcu.h b/include/misc/xlnx_vcu.h
new file mode 100644
index 0000000..ca33333
--- /dev/null
+++ b/include/misc/xlnx_vcu.h
@@ -0,0 +1,18 @@
+/*
+ * Xilinx VCU Init
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Contacts Dhaval Shah <[email protected]>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+#ifndef _XILINX_VCU_H_
+#define _XILINX_VCU_H_
+
+struct xvcu_device;
+
+u32 xvcu_get_color_depth(struct xvcu_device *xvcu);
+u32 xvcu_get_memory_depth(struct xvcu_device *xvcu);
+
+#endif /* _XILINX_VCU_H_ */
--
2.7.4


2017-12-05 12:18:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]> wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah <[email protected]>

Can you explain what a "VCU" is and why there is no existing subsystem for it?

If this is the "video codec unit", I'd suggest moving it to
drivers/media/platform
and sending the patch to the [email protected] list instead.

Arnd

2017-12-05 12:38:29

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Arnd,

Thanks a lot for the review.
Replies inline.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Arnd Bergmann
Sent: Tuesday, December 05, 2017 4:19 AM
To: Dhaval Rajeshbhai Shah <[email protected]>
Cc: gregkh <[email protected]>; Linux Kernel Mailing List <[email protected]>; Michal Simek <[email protected]>; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai Shah <[email protected]>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]> wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used by the
> encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah <[email protected]>

Can you explain what a "VCU" is and why there is no existing subsystem for it?

[Dhaval] : This VCU means Video codec unit. Here, this driver is for the logicoreIP and not the VCU which is created to support the PS and PL isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

If this is the "video codec unit", I'd suggest moving it to drivers/media/platform and sending the patch to the [email protected] list instead.
[Dhaval] : This is not the Video codec unit but the driver to provide the support the IP created for the PS and PL isolation and clock related information. That why I have placed this under the "drivers/misc/".


Arnd

2017-12-05 13:29:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the
[PATCH] one :)

>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
> drivers/misc/Kconfig | 6 +
> drivers/misc/Makefile | 1 +
> drivers/misc/xlnx_vcu.c | 664 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/misc/xlnx_vcu.h | 18 ++
> 4 files changed, 689 insertions(+)
> create mode 100644 drivers/misc/xlnx_vcu.c
> create mode 100644 include/misc/xlnx_vcu.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test driver
> for PCI Endpoint.
>
> +config XILINX_VCU
> + tristate "Xilinx VCU Init"
> + default n

That's always the default, no need for this.

> + help
> + Driver for the Xilinx VCU Init based on the logicoreIP.

You need a lot more help text here to explain what this driver is, what
it is for, and who would need it.

Also, why is this a misc driver?

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts Dhaval Shah <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.


> + */
> +#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>
> +
> +#include <misc/xlnx_vcu.h>

Why do you need a .h file for a single driver?

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu: Pointer to the xvcu_device structure
> + *
> + * Return: Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu)
> +{
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH);
> +}
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...

greg k-h

2017-12-05 13:32:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah <[email protected]> wrote:
> From: [email protected] [mailto:[email protected]] On Behalf Of Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: gregkh <[email protected]>; Linux Kernel Mailing List <[email protected]>; Michal Simek <[email protected]>; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai Shah <[email protected]>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]> wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
>> created. This driver will provide the api which can be used by the
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah <[email protected]>
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the logicoreIP and not the VCU which is created to support the PS and PL isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

What are PS and PL then?

> If this is the "video codec unit", I'd suggest moving it to drivers/media/platform and sending the patch to the [email protected] list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the support the IP created for the PS and PL isolation and clock related information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will
be answered when
you explain PS and PL). If not, just put it in the same directory as the codec.

Arnd

2017-12-06 05:44:52

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Arnd,

Replies inline.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Arnd Bergmann
Sent: Tuesday, December 05, 2017 5:32 AM
To: Dhaval Rajeshbhai Shah <[email protected]>
Cc: gregkh <[email protected]>; Linux Kernel Mailing List <[email protected]>; Michal Simek <[email protected]>; Hyun Kwon <[email protected]>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah <[email protected]> wrote:
> From: [email protected] [mailto:[email protected]] On Behalf
> Of Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: gregkh <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; Michal Simek
> <[email protected]>; Hyun Kwon <[email protected]>; Dhaval
> Rajeshbhai Shah <[email protected]>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]> wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
>> created. This driver will provide the api which can be used by the
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah <[email protected]>
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the logicoreIP and not the VCU which is created to support the PS and PL isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

What are PS and PL then?
[Dhaval ] : PS is the Processing system. PL is the Programmable logic. There are few register in the logicoreIP which provides the isolation between this two in the chip.

> If this is the "video codec unit", I'd suggest moving it to drivers/media/platform and sending the patch to the [email protected] list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the support the IP created for the PS and PL isolation and clock related information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will be answered when you explain PS and PL). If not, just put it in the same directory as the codec.
[Dhaval ] : Yes. Driver is useful without a codec driver as well. No dependency on the Codec driver.

Arnd

2017-12-06 06:01:43

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Hi Greg k-h,

Thanks a lot for the review.

Replies inline.

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Tuesday, December 05, 2017 5:30 AM
To: Dhaval Rajeshbhai Shah <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Hyun Kwon <[email protected]>; Dhaval Rajeshbhai Shah <[email protected]>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used by the
> encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the [PATCH] one :)
[Dhaval ] : I will take care from next version.

>
> Signed-off-by: Dhaval Shah <[email protected]>
> ---
> drivers/misc/Kconfig | 6 +
> drivers/misc/Makefile | 1 +
> drivers/misc/xlnx_vcu.c | 664
> ++++++++++++++++++++++++++++++++++++++++++++++++
> include/misc/xlnx_vcu.h | 18 ++
> 4 files changed, 689 insertions(+)
> create mode 100644 drivers/misc/xlnx_vcu.c create mode 100644
> include/misc/xlnx_vcu.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test driver
> for PCI Endpoint.
>
> +config XILINX_VCU
> + tristate "Xilinx VCU Init"
> + default n

That's always the default, no need for this.
[Dhaval ] : I will remove that.

> + help
> + Driver for the Xilinx VCU Init based on the logicoreIP.

You need a lot more help text here to explain what this driver is, what it is for, and who would need it.
[Dhaval ] : I will provide more help text to provide more help on driver.

Also, why is this a misc driver?
[Dhaval ] : this driver is for the logicoreIP which is created to support the Processing system and Programmable logic isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts Dhaval Shah <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.
[Dhaval ] : I will update that SPDX license as you said.


> + */
> +#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>
> +
> +#include <misc/xlnx_vcu.h>

Why do you need a .h file for a single driver?
[Dhaval ] : There are few APIs and structure which are provided in the .h file which will be used by the other driver on the Xilinx based system. I have exported few API as well because of this reason. Those API will share the information from the logicoreIP register set.

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu: Pointer to the xvcu_device structure
> + *
> + * Return: Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu) {
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH); }
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...
[Dhaval ] : There are few information from the logicoreIp register set which needs to share with other driver. That's why those API are exported to use by other driver to get those information based on the requirement.

greg k-h

Regards,
Dhaval

2017-12-06 07:55:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Wed, Dec 06, 2017 at 06:01:37AM +0000, Dhaval Rajeshbhai Shah wrote:
> Hi Greg k-h,
>
> Thanks a lot for the review.
>
> Replies inline.

As they should be, perhaps you need a better email client :)

>
> > +config XILINX_VCU
> > + tristate "Xilinx VCU Init"
> > + default n
>
> That's always the default, no need for this.
> [Dhaval ] : I will remove that.

This style of replying is very odd, please use the normal format in the
future.

> > + help
> > + Driver for the Xilinx VCU Init based on the logicoreIP.
>
> You need a lot more help text here to explain what this driver is, what it is for, and who would need it.
> [Dhaval ] : I will provide more help text to provide more help on driver.
>
> Also, why is this a misc driver?
> [Dhaval ] : this driver is for the logicoreIP which is created to support the Processing system and Programmable logic isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

Then you need to explain this a lot better, posting a random driver
for submission that is expected to be used by another one isn't ok.
Post the whole patch series please, we do not add infrastructure to the
kernel that is not used right then.

thanks,

greg k-h

2017-12-06 09:06:01

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver



> -----Original Message-----
> From: 'Greg KH' [mailto:[email protected]]
> Sent: Tuesday, December 05, 2017 11:56 PM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Hyun Kwon <[email protected]>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
>
> On Wed, Dec 06, 2017 at 06:01:37AM +0000, Dhaval Rajeshbhai Shah wrote:
> > Hi Greg k-h,
> >
> > Thanks a lot for the review.
> >
> > Replies inline.
>
> As they should be, perhaps you need a better email client :)
>
> >
> > > +config XILINX_VCU
> > > + tristate "Xilinx VCU Init"
> > > + default n
> >
> > That's always the default, no need for this.
> > [Dhaval ] : I will remove that.
>
> This style of replying is very odd, please use the normal format in the future.
I have updated that in this reply.
>
> > > + help
> > > + Driver for the Xilinx VCU Init based on the logicoreIP.
> >
> > You need a lot more help text here to explain what this driver is, what it is
> for, and who would need it.
> > [Dhaval ] : I will provide more help text to provide more help on driver.
> >
> > Also, why is this a misc driver?
> > [Dhaval ] : this driver is for the logicoreIP which is created to support the
> Processing system and Programmable logic isolation and to provide the clock
> related information. So this is not a VCU driver and but just a intermediate
> driver which supports logicoreIP. That's why no subsystem for this.
>
> Then you need to explain this a lot better, posting a random driver for
> submission that is expected to be used by another one isn't ok.
> Post the whole patch series please, we do not add infrastructure to the
> kernel that is not used right then.
Can I remove the export api and header file and make this driver generic for the logicoreIP? No other driver will depend on that. This will help us to remove the isolation between the Programmable system and Programmable logic by configuring the logicoreIp register set.

Thanks
Dhaval
>
> thanks,
>
> greg k-h

2017-12-06 09:15:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Wed, Dec 06, 2017 at 09:05:51AM +0000, Dhaval Rajeshbhai Shah wrote:
> > Then you need to explain this a lot better, posting a random driver for
> > submission that is expected to be used by another one isn't ok.
> > Post the whole patch series please, we do not add infrastructure to the
> > kernel that is not used right then.
> Can I remove the export api and header file and make this driver generic for the logicoreIP? No other driver will depend on that. This will help us to remove the isolation between the Programmable system and Programmable logic by configuring the logicoreIp register set.

I don't know, let's see what that patch submission looks like. If it
doesn't do anything, that's not really a good idea :)

What's wrong with just submitting your entire driver series? Why break
it up like this? That's normally not a good idea, you want us to review
it in a "whole" idea, not piece-meal.

thanks,

greg k-h

2017-12-06 09:38:14

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver



> -----Original Message-----
> From: 'Greg KH' [mailto:[email protected]]
> Sent: Wednesday, December 06, 2017 1:16 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Hyun Kwon <[email protected]>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
>
> On Wed, Dec 06, 2017 at 09:05:51AM +0000, Dhaval Rajeshbhai Shah wrote:
> > > Then you need to explain this a lot better, posting a random driver
> > > for submission that is expected to be used by another one isn't ok.
> > > Post the whole patch series please, we do not add infrastructure to
> > > the kernel that is not used right then.
> > Can I remove the export api and header file and make this driver generic
> for the logicoreIP? No other driver will depend on that. This will help us to
> remove the isolation between the Programmable system and Programmable
> logic by configuring the logicoreIp register set.
>
> I don't know, let's see what that patch submission looks like. If it doesn't do
> anything, that's not really a good idea :)
>
This will do the isolation enable and removal related stuff along with the set the proper frequency based on the clock information get from the logicoreIp. In this way, This is very helpful driver.

> What's wrong with just submitting your entire driver series? Why break it up
> like this? That's normally not a good idea, you want us to review it in a
> "whole" idea, not piece-meal.
Codecs are owned by different team. when they will submit their code they will extend this driver.

Thanks,
Dhaval
>
> thanks,
>
> greg k-h

2017-12-07 10:07:27

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

Daval,

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]> wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah <[email protected]>
[]
> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 0000000..373f7f9
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts Dhaval Shah <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */

The SPDX id should be on the first line with a C++ // style comment
instead as requested by Linus.
--
Cordially
Philippe Ombredanne

2017-12-07 10:31:07

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver



> -----Original Message-----
> From: Philippe Ombredanne [mailto:[email protected]]
> Sent: Thursday, December 07, 2017 2:07 AM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; LKML <[email protected]>;
> [email protected]; Hyun Kwon <[email protected]>; Dhaval
> Rajeshbhai Shah <[email protected]>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
>
> Daval,
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]>
> wrote:
> > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > created. This driver will provide the api which can be used by the
> > encoder and decoder driver to get the configured value.
> >
> > Signed-off-by: Dhaval Shah <[email protected]>
> []
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 0000000..373f7f9
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,664 @@
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts Dhaval Shah <[email protected]>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
>
> The SPDX id should be on the first line with a C++ // style comment instead as
> requested by Linus.
I will take care of this Changes as also suggested by the Greg k-h as well once get the ACK for the remaining reply from the Greg k-h.

Thanks & Regards,
Dhaval
> --
> Cordially
> Philippe Ombredanne

2017-12-07 10:47:52

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

On Thu, Dec 07, 2017 at 10:30:45AM +0000, Dhaval Rajeshbhai Shah wrote:
>
>
> > -----Original Message-----
> > From: Philippe Ombredanne [mailto:[email protected]]
> > Sent: Thursday, December 07, 2017 2:07 AM
> > To: Dhaval Rajeshbhai Shah <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> > <[email protected]>; LKML <[email protected]>;
> > [email protected]; Hyun Kwon <[email protected]>; Dhaval
> > Rajeshbhai Shah <[email protected]>
> > Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> > logicoreIP init driver
> >
> > Daval,
> >
> > On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <[email protected]>
> > wrote:
> > > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > > created. This driver will provide the api which can be used by the
> > > encoder and decoder driver to get the configured value.
> > >
> > > Signed-off-by: Dhaval Shah <[email protected]>
> > []
> > > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > > file mode 100644 index 0000000..373f7f9
> > > --- /dev/null
> > > +++ b/drivers/misc/xlnx_vcu.c
> > > @@ -0,0 +1,664 @@
> > > +/*
> > > + * Xilinx VCU Init
> > > + *
> > > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > > + *
> > > + * Contacts Dhaval Shah <[email protected]>
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0
> > > + */
> >
> > The SPDX id should be on the first line with a C++ // style comment instead as
> > requested by Linus.
> I will take care of this Changes as also suggested by the Greg k-h as well once get the ACK for the remaining reply from the Greg k-h.

Don't wait for me, my queue is really long, if you have fixes that you
know you need to make, make them!

greg k-h

2017-12-07 21:43:46

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v2 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-07 21:43:58

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v2 1/2] Documentation: devicetree: 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]>
---
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-07 21:44:11

by Dhaval Shah

[permalink] [raw]
Subject: [PATCH v2 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 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-07 21:47:49

by Philippe Ombredanne

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

Dear Dhaval,

On Thu, Dec 7, 2017 at 10:31 PM, 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]>
> ---
> 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.

Thank you for the SPDX comments updates.

Acked-by: Philippe Ombredanne <[email protected]>

--
Cordially
Philippe Ombredanne

2017-12-12 20:07:08

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Documentation: devicetree: Add DT bindings to xlnx_vcu driver

On Thu, Dec 07, 2017 at 01:31:15PM -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]>
> ---
> 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]>

One nit. Use "dt-bindings: misc: ..." for the subject.

2017-12-13 05:12:34

by Dhaval Rajeshbhai Shah

[permalink] [raw]
Subject: RE: [PATCH v2 1/2] Documentation: devicetree: Add DT bindings to xlnx_vcu driver

Hi Rob,

Thanks a lot for the review.

> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: Tuesday, December 12, 2017 12:07 PM
> To: Dhaval Rajeshbhai Shah <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hyun Kwon <[email protected]>; Dhaval
> Rajeshbhai Shah <[email protected]>
> Subject: Re: [PATCH v2 1/2] Documentation: devicetree: Add DT bindings to
> xlnx_vcu driver
>
> On Thu, Dec 07, 2017 at 01:31:15PM -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]>
> > ---
> > 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]>
>
> One nit. Use "dt-bindings: misc: ..." for the subject.
I will update subject line and send you v3 patch set.