2019-06-09 00:07:43

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state

Stores configuration based on parameters from the DT
node and values from the SD-FEC core plus reads
the default state from the SD-FEC core. To obtain
values from the core register read, write capabilities
have been added plus related register map details.

Tested-by: Dragan Cvetic <[email protected]>
Signed-off-by: Derek Kiernan <[email protected]>
Signed-off-by: Dragan Cvetic <[email protected]>
---
drivers/misc/xilinx_sdfec.c | 293 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 138 ++++++++++++++++++
2 files changed, 431 insertions(+)
create mode 100644 include/uapi/misc/xilinx_sdfec.h

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index 4524677..ddfca54 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -20,8 +20,89 @@
#include <linux/slab.h>
#include <linux/clk.h>

+#include <uapi/misc/xilinx_sdfec.h>
+
static int xsdfec_ndevs;

+/* Xilinx SDFEC Register Map */
+/* CODE_WRI_PROTECT Register */
+#define XSDFEC_CODE_WR_PROTECT_ADDR (0x4)
+
+/* ACTIVE Register */
+#define XSDFEC_ACTIVE_ADDR (0x8)
+#define XSDFEC_IS_ACTIVITY_SET (0x1)
+
+/* AXIS_WIDTH Register */
+#define XSDFEC_AXIS_WIDTH_ADDR (0xC)
+#define XSDFEC_AXIS_DOUT_WORDS_LSB (5)
+#define XSDFEC_AXIS_DOUT_WIDTH_LSB (3)
+#define XSDFEC_AXIS_DIN_WORDS_LSB (2)
+#define XSDFEC_AXIS_DIN_WIDTH_LSB (0)
+
+/* AXIS_ENABLE Register */
+#define XSDFEC_AXIS_ENABLE_ADDR (0x10)
+#define XSDFEC_AXIS_OUT_ENABLE_MASK (0x38)
+#define XSDFEC_AXIS_IN_ENABLE_MASK (0x7)
+#define XSDFEC_AXIS_ENABLE_MASK \
+ (XSDFEC_AXIS_OUT_ENABLE_MASK | XSDFEC_AXIS_IN_ENABLE_MASK)
+
+/* FEC_CODE Register */
+#define XSDFEC_FEC_CODE_ADDR (0x14)
+
+/* ORDER Register Map */
+#define XSDFEC_ORDER_ADDR (0x18)
+
+/* Interrupt Status Register */
+#define XSDFEC_ISR_ADDR (0x1C)
+/* Interrupt Status Register Bit Mask */
+#define XSDFEC_ISR_MASK (0x3F)
+
+/* Write Only - Interrupt Enable Register */
+#define XSDFEC_IER_ADDR (0x20)
+/* Write Only - Interrupt Disable Register */
+#define XSDFEC_IDR_ADDR (0x24)
+/* Read Only - Interrupt Mask Register */
+#define XSDFEC_IMR_ADDR (0x28)
+
+/* ECC Interrupt Status Register */
+#define XSDFEC_ECC_ISR_ADDR (0x2C)
+/* Single Bit Errors */
+#define XSDFEC_ECC_ISR_SBE_MASK (0x7FF)
+/* PL Initialize Single Bit Errors */
+#define XSDFEC_PL_INIT_ECC_ISR_SBE_MASK (0x3C00000)
+/* Multi Bit Errors */
+#define XSDFEC_ECC_ISR_MBE_MASK (0x3FF800)
+/* PL Initialize Multi Bit Errors */
+#define XSDFEC_PL_INIT_ECC_ISR_MBE_MASK (0x3C000000)
+/* Multi Bit Error to Event Shift */
+#define XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT (11)
+/* PL Initialize Multi Bit Error to Event Shift */
+#define XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT (4)
+/* ECC Interrupt Status Bit Mask */
+#define XSDFEC_ECC_ISR_MASK (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_ECC_ISR_MBE_MASK)
+/* ECC Interrupt Status PL Initialize Bit Mask */
+#define XSDFEC_PL_INIT_ECC_ISR_MASK \
+ (XSDFEC_PL_INIT_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
+/* ECC Interrupt Status All Bit Mask */
+#define XSDFEC_ALL_ECC_ISR_MASK \
+ (XSDFEC_ECC_ISR_MASK | XSDFEC_PL_INIT_ECC_ISR_MASK)
+/* ECC Interrupt Status Single Bit Errors Mask */
+#define XSDFEC_ALL_ECC_ISR_SBE_MASK \
+ (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_SBE_MASK)
+/* ECC Interrupt Status Multi Bit Errors Mask */
+#define XSDFEC_ALL_ECC_ISR_MBE_MASK \
+ (XSDFEC_ECC_ISR_MBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
+
+/* Write Only - ECC Interrupt Enable Register */
+#define XSDFEC_ECC_IER_ADDR (0x30)
+/* Write Only - ECC Interrupt Disable Register */
+#define XSDFEC_ECC_IDR_ADDR (0x34)
+/* Read Only - ECC Interrupt Mask Register */
+#define XSDFEC_ECC_IMR_ADDR (0x38)
+
+/* BYPASS Register */
+#define XSDFEC_BYPASS_ADDR (0x3C)
+
/**
* struct xsdfec_clks - For managing SD-FEC clocks
* @core_clk: Main processing clock for core
@@ -48,6 +129,8 @@ struct xsdfec_clks {
* struct xsdfec_dev - Driver data for SDFEC
* @regs: device physical base address
* @dev: pointer to device struct
+ * @state: State of the SDFEC device
+ * @config: Configuration of the SDFEC device
* @miscdev: Misc device handle
* @error_data_lock: Error counter and states spinlock
* @clks: Clocks managed by the SDFEC driver
@@ -57,16 +140,220 @@ struct xsdfec_clks {
struct xsdfec_dev {
void __iomem *regs;
struct device *dev;
+ enum xsdfec_state state;
+ struct xsdfec_config config;
struct miscdevice miscdev;
/* Spinlock to protect state_updated and stats_updated */
spinlock_t error_data_lock;
struct xsdfec_clks clks;
};

+static inline void xsdfec_regwrite(struct xsdfec_dev *xsdfec, u32 addr,
+ u32 value)
+{
+ dev_dbg(xsdfec->dev, "Writing 0x%x to offset 0x%x", value, addr);
+ iowrite32(value, xsdfec->regs + addr);
+}
+
+static inline u32 xsdfec_regread(struct xsdfec_dev *xsdfec, u32 addr)
+{
+ u32 rval;
+
+ rval = ioread32(xsdfec->regs + addr);
+ dev_dbg(xsdfec->dev, "Read value = 0x%x from offset 0x%x", rval, addr);
+ return rval;
+}
+
+static void update_bool_config_from_reg(struct xsdfec_dev *xsdfec,
+ u32 reg_offset, u32 bit_num,
+ char *config_value)
+{
+ u32 reg_val;
+ u32 bit_mask = 1 << bit_num;
+
+ reg_val = xsdfec_regread(xsdfec, reg_offset);
+ *config_value = (reg_val & bit_mask) > 0;
+}
+
+static void update_config_from_hw(struct xsdfec_dev *xsdfec)
+{
+ u32 reg_value;
+ bool sdfec_started;
+
+ /* Update the Order */
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_ORDER_ADDR);
+ xsdfec->config.order = reg_value;
+
+ update_bool_config_from_reg(xsdfec, XSDFEC_BYPASS_ADDR,
+ 0, /* Bit Number, maybe change to mask */
+ &xsdfec->config.bypass);
+
+ update_bool_config_from_reg(xsdfec, XSDFEC_CODE_WR_PROTECT_ADDR,
+ 0, /* Bit Number */
+ &xsdfec->config.code_wr_protect);
+
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
+ xsdfec->config.irq.enable_isr = (reg_value & XSDFEC_ISR_MASK) > 0;
+
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
+ xsdfec->config.irq.enable_ecc_isr =
+ (reg_value & XSDFEC_ECC_ISR_MASK) > 0;
+
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_AXIS_ENABLE_ADDR);
+ sdfec_started = (reg_value & XSDFEC_AXIS_IN_ENABLE_MASK) > 0;
+ if (sdfec_started)
+ xsdfec->state = XSDFEC_STARTED;
+ else
+ xsdfec->state = XSDFEC_STOPPED;
+}
+
+static u32
+xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
+{
+ u32 axis_width_field = 0;
+
+ switch (axis_width_cfg) {
+ case XSDFEC_1x128b:
+ axis_width_field = 0;
+ break;
+ case XSDFEC_2x128b:
+ axis_width_field = 1;
+ break;
+ case XSDFEC_4x128b:
+ axis_width_field = 2;
+ break;
+ }
+
+ return axis_width_field;
+}
+
+static u32 xsdfec_translate_axis_words_cfg_val(enum xsdfec_axis_word_include
+ axis_word_inc_cfg)
+{
+ u32 axis_words_field = 0;
+
+ if (axis_word_inc_cfg == XSDFEC_FIXED_VALUE ||
+ axis_word_inc_cfg == XSDFEC_IN_BLOCK)
+ axis_words_field = 0;
+ else if (axis_word_inc_cfg == XSDFEC_PER_AXI_TRANSACTION)
+ axis_words_field = 1;
+
+ return axis_words_field;
+}
+
+static int xsdfec_cfg_axi_streams(struct xsdfec_dev *xsdfec)
+{
+ u32 reg_value;
+ u32 dout_words_field;
+ u32 dout_width_field;
+ u32 din_words_field;
+ u32 din_width_field;
+ struct xsdfec_config *config = &xsdfec->config;
+
+ /* translate config info to register values */
+ dout_words_field =
+ xsdfec_translate_axis_words_cfg_val(config->dout_word_include);
+ dout_width_field =
+ xsdfec_translate_axis_width_cfg_val(config->dout_width);
+ din_words_field =
+ xsdfec_translate_axis_words_cfg_val(config->din_word_include);
+ din_width_field =
+ xsdfec_translate_axis_width_cfg_val(config->din_width);
+
+ reg_value = dout_words_field << XSDFEC_AXIS_DOUT_WORDS_LSB;
+ reg_value |= dout_width_field << XSDFEC_AXIS_DOUT_WIDTH_LSB;
+ reg_value |= din_words_field << XSDFEC_AXIS_DIN_WORDS_LSB;
+ reg_value |= din_width_field << XSDFEC_AXIS_DIN_WIDTH_LSB;
+
+ xsdfec_regwrite(xsdfec, XSDFEC_AXIS_WIDTH_ADDR, reg_value);
+
+ return 0;
+}
+
static const struct file_operations xsdfec_fops = {
.owner = THIS_MODULE,
};

+static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
+{
+ struct device *dev = xsdfec->dev;
+ struct device_node *node = dev->of_node;
+ int rval;
+ const char *fec_code;
+ u32 din_width;
+ u32 din_word_include;
+ u32 dout_width;
+ u32 dout_word_include;
+
+ rval = of_property_read_string(node, "xlnx,sdfec-code", &fec_code);
+ if (rval < 0)
+ return rval;
+
+ if (!strcasecmp(fec_code, "ldpc"))
+ xsdfec->config.code = XSDFEC_LDPC_CODE;
+ else if (!strcasecmp(fec_code, "turbo"))
+ xsdfec->config.code = XSDFEC_TURBO_CODE;
+ else
+ return -EINVAL;
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-din-words",
+ &din_word_include);
+ if (rval < 0)
+ return rval;
+
+ if (din_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
+ xsdfec->config.din_word_include = din_word_include;
+ else
+ return -EINVAL;
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-din-width", &din_width);
+ if (rval < 0)
+ return rval;
+
+ switch (din_width) {
+ /* Fall through and set for valid values */
+ case XSDFEC_1x128b:
+ case XSDFEC_2x128b:
+ case XSDFEC_4x128b:
+ xsdfec->config.din_width = din_width;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-dout-words",
+ &dout_word_include);
+ if (rval < 0)
+ return rval;
+
+ if (dout_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
+ xsdfec->config.dout_word_include = dout_word_include;
+ else
+ return -EINVAL;
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-dout-width", &dout_width);
+ if (rval < 0)
+ return rval;
+
+ switch (dout_width) {
+ /* Fall through and set for valid values */
+ case XSDFEC_1x128b:
+ case XSDFEC_2x128b:
+ case XSDFEC_4x128b:
+ xsdfec->config.dout_width = dout_width;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Write LDPC to CODE Register */
+ xsdfec_regwrite(xsdfec, XSDFEC_FEC_CODE_ADDR, xsdfec->config.code);
+
+ xsdfec_cfg_axi_streams(xsdfec);
+
+ return 0;
+}
+
static int xsdfec_clk_init(struct platform_device *pdev,
struct xsdfec_clks *clks)
{
@@ -247,6 +534,12 @@ static int xsdfec_probe(struct platform_device *pdev)
goto err_xsdfec_dev;
}

+ err = xsdfec_parse_of(xsdfec);
+ if (err < 0)
+ goto err_xsdfec_dev;
+
+ update_config_from_hw(xsdfec);
+
/* Save driver private data */
platform_set_drvdata(pdev, xsdfec);

diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
new file mode 100644
index 0000000..7b47a4c
--- /dev/null
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -0,0 +1,138 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Xilinx SD-FEC
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Description:
+ * This driver is developed for SDFEC16 IP. It provides a char device
+ * in sysfs and supports file operations like open(), close() and ioctl().
+ */
+#ifndef __XILINX_SDFEC_H__
+#define __XILINX_SDFEC_H__
+
+#include <linux/types.h>
+
+/**
+ * enum xsdfec_code - Code Type.
+ * @XSDFEC_TURBO_CODE: Driver is configured for Turbo mode.
+ * @XSDFEC_LDPC_CODE: Driver is configured for LDPC mode.
+ *
+ * This enum is used to indicate the mode of the driver. The mode is determined
+ * by checking which codes are set in the driver. Note that the mode cannot be
+ * changed by the driver.
+ */
+enum xsdfec_code {
+ XSDFEC_TURBO_CODE = 0,
+ XSDFEC_LDPC_CODE,
+};
+
+/**
+ * enum xsdfec_order - Order
+ * @XSDFEC_MAINTAIN_ORDER: Maintain order execution of blocks.
+ * @XSDFEC_OUT_OF_ORDER: Out-of-order execution of blocks.
+ *
+ * This enum is used to indicate whether the order of blocks can change from
+ * input to output.
+ */
+enum xsdfec_order {
+ XSDFEC_MAINTAIN_ORDER = 0,
+ XSDFEC_OUT_OF_ORDER,
+};
+
+/**
+ * enum xsdfec_state - State.
+ * @XSDFEC_INIT: Driver is initialized.
+ * @XSDFEC_STARTED: Driver is started.
+ * @XSDFEC_STOPPED: Driver is stopped.
+ * @XSDFEC_NEEDS_RESET: Driver needs to be reset.
+ * @XSDFEC_PL_RECONFIGURE: Programmable Logic needs to be recofigured.
+ *
+ * This enum is used to indicate the state of the driver.
+ */
+enum xsdfec_state {
+ XSDFEC_INIT = 0,
+ XSDFEC_STARTED,
+ XSDFEC_STOPPED,
+ XSDFEC_NEEDS_RESET,
+ XSDFEC_PL_RECONFIGURE,
+};
+
+/**
+ * enum xsdfec_axis_width - AXIS_WIDTH.DIN Setting for 128-bit width.
+ * @XSDFEC_1x128b: DIN data input stream consists of a 128-bit lane
+ * @XSDFEC_2x128b: DIN data input stream consists of two 128-bit lanes
+ * @XSDFEC_4x128b: DIN data input stream consists of four 128-bit lanes
+ *
+ * This enum is used to indicate the AXIS_WIDTH.DIN setting for 128-bit width.
+ * The number of lanes of the DIN data input stream depends upon the
+ * AXIS_WIDTH.DIN parameter.
+ */
+enum xsdfec_axis_width {
+ XSDFEC_1x128b = 1,
+ XSDFEC_2x128b = 2,
+ XSDFEC_4x128b = 4,
+};
+
+/**
+ * enum xsdfec_axis_word_include - Words Configuration.
+ * @XSDFEC_FIXED_VALUE: Fixed, the DIN_WORDS AXI4-Stream interface is removed
+ * from the IP instance and is driven with the specified
+ * number of words.
+ * @XSDFEC_IN_BLOCK: In Block, configures the IP instance to expect a single
+ * DIN_WORDS value per input code block. The DIN_WORDS
+ * interface is present.
+ * @XSDFEC_PER_AXI_TRANSACTION: Per Transaction, configures the IP instance to
+ * expect one DIN_WORDS value per input transaction on the DIN interface. The
+ * DIN_WORDS interface is present.
+ * @XSDFEC_AXIS_WORDS_INCLUDE_MAX: Used to indicate out of bound Words
+ * Configurations.
+ *
+ * This enum is used to specify the DIN_WORDS configuration.
+ */
+enum xsdfec_axis_word_include {
+ XSDFEC_FIXED_VALUE = 0,
+ XSDFEC_IN_BLOCK,
+ XSDFEC_PER_AXI_TRANSACTION,
+ XSDFEC_AXIS_WORDS_INCLUDE_MAX,
+};
+
+/**
+ * struct xsdfec_irq - Enabling or Disabling Interrupts.
+ * @enable_isr: If true enables the ISR
+ * @enable_ecc_isr: If true enables the ECC ISR
+ */
+struct xsdfec_irq {
+ __s8 enable_isr;
+ __s8 enable_ecc_isr;
+};
+
+/**
+ * struct xsdfec_config - Configuration of SD-FEC core.
+ * @code: The codes being used by the SD-FEC instance
+ * @order: Order of Operation
+ * @bypass: Is the core being bypassed
+ * @code_wr_protect: Is write protection of LDPC codes enabled
+ * @din_width: Width of the DIN AXI4-Stream
+ * @din_word_include: How DIN_WORDS are inputted
+ * @dout_width: Width of the DOUT AXI4-Stream
+ * @dout_word_include: HOW DOUT_WORDS are outputted
+ * @irq: Enabling or disabling interrupts
+ */
+struct xsdfec_config {
+ enum xsdfec_code code;
+ enum xsdfec_order order;
+ __s8 bypass;
+ __s8 code_wr_protect;
+ enum xsdfec_axis_width din_width;
+ enum xsdfec_axis_word_include din_word_include;
+ enum xsdfec_axis_width dout_width;
+ enum xsdfec_axis_word_include dout_word_include;
+ struct xsdfec_irq irq;
+};
+
+/*
+ * XSDFEC IOCTL List
+ */
+#define XSDFEC_MAGIC 'f'
+#endif /* __XILINX_SDFEC_H__ */
--
2.7.4


2019-06-09 11:42:48

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state

On Sun, Jun 09, 2019 at 01:04:09AM +0100, Dragan Cvetic wrote:
> Stores configuration based on parameters from the DT
> node and values from the SD-FEC core plus reads
> the default state from the SD-FEC core. To obtain
> values from the core register read, write capabilities
> have been added plus related register map details.
>
> Tested-by: Dragan Cvetic <[email protected]>
> Signed-off-by: Derek Kiernan <[email protected]>
> Signed-off-by: Dragan Cvetic <[email protected]>
> ---
> drivers/misc/xilinx_sdfec.c | 293 +++++++++++++++++++++++++++++++++++++++
> include/uapi/misc/xilinx_sdfec.h | 138 ++++++++++++++++++
> 2 files changed, 431 insertions(+)
> create mode 100644 include/uapi/misc/xilinx_sdfec.h
>
> diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> index 4524677..ddfca54 100644
> --- a/drivers/misc/xilinx_sdfec.c
> +++ b/drivers/misc/xilinx_sdfec.c
> @@ -20,8 +20,89 @@
> #include <linux/slab.h>
> #include <linux/clk.h>
>
> +#include <uapi/misc/xilinx_sdfec.h>
> +
> static int xsdfec_ndevs;
>
> +/* Xilinx SDFEC Register Map */
> +/* CODE_WRI_PROTECT Register */
> +#define XSDFEC_CODE_WR_PROTECT_ADDR (0x4)
> +
> +/* ACTIVE Register */
> +#define XSDFEC_ACTIVE_ADDR (0x8)
> +#define XSDFEC_IS_ACTIVITY_SET (0x1)
> +
> +/* AXIS_WIDTH Register */
> +#define XSDFEC_AXIS_WIDTH_ADDR (0xC)
> +#define XSDFEC_AXIS_DOUT_WORDS_LSB (5)
> +#define XSDFEC_AXIS_DOUT_WIDTH_LSB (3)
> +#define XSDFEC_AXIS_DIN_WORDS_LSB (2)
> +#define XSDFEC_AXIS_DIN_WIDTH_LSB (0)
> +
> +/* AXIS_ENABLE Register */
> +#define XSDFEC_AXIS_ENABLE_ADDR (0x10)
> +#define XSDFEC_AXIS_OUT_ENABLE_MASK (0x38)
> +#define XSDFEC_AXIS_IN_ENABLE_MASK (0x7)
> +#define XSDFEC_AXIS_ENABLE_MASK \
> + (XSDFEC_AXIS_OUT_ENABLE_MASK | XSDFEC_AXIS_IN_ENABLE_MASK)
> +
> +/* FEC_CODE Register */
> +#define XSDFEC_FEC_CODE_ADDR (0x14)
> +
> +/* ORDER Register Map */
> +#define XSDFEC_ORDER_ADDR (0x18)
> +
> +/* Interrupt Status Register */
> +#define XSDFEC_ISR_ADDR (0x1C)
> +/* Interrupt Status Register Bit Mask */
> +#define XSDFEC_ISR_MASK (0x3F)
> +
> +/* Write Only - Interrupt Enable Register */
> +#define XSDFEC_IER_ADDR (0x20)
> +/* Write Only - Interrupt Disable Register */
> +#define XSDFEC_IDR_ADDR (0x24)
> +/* Read Only - Interrupt Mask Register */
> +#define XSDFEC_IMR_ADDR (0x28)
> +
> +/* ECC Interrupt Status Register */
> +#define XSDFEC_ECC_ISR_ADDR (0x2C)
> +/* Single Bit Errors */
> +#define XSDFEC_ECC_ISR_SBE_MASK (0x7FF)
> +/* PL Initialize Single Bit Errors */
> +#define XSDFEC_PL_INIT_ECC_ISR_SBE_MASK (0x3C00000)
> +/* Multi Bit Errors */
> +#define XSDFEC_ECC_ISR_MBE_MASK (0x3FF800)
> +/* PL Initialize Multi Bit Errors */
> +#define XSDFEC_PL_INIT_ECC_ISR_MBE_MASK (0x3C000000)
> +/* Multi Bit Error to Event Shift */
> +#define XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT (11)
> +/* PL Initialize Multi Bit Error to Event Shift */
> +#define XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT (4)
> +/* ECC Interrupt Status Bit Mask */
> +#define XSDFEC_ECC_ISR_MASK (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_ECC_ISR_MBE_MASK)
> +/* ECC Interrupt Status PL Initialize Bit Mask */
> +#define XSDFEC_PL_INIT_ECC_ISR_MASK \
> + (XSDFEC_PL_INIT_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> +/* ECC Interrupt Status All Bit Mask */
> +#define XSDFEC_ALL_ECC_ISR_MASK \
> + (XSDFEC_ECC_ISR_MASK | XSDFEC_PL_INIT_ECC_ISR_MASK)
> +/* ECC Interrupt Status Single Bit Errors Mask */
> +#define XSDFEC_ALL_ECC_ISR_SBE_MASK \
> + (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_SBE_MASK)
> +/* ECC Interrupt Status Multi Bit Errors Mask */
> +#define XSDFEC_ALL_ECC_ISR_MBE_MASK \
> + (XSDFEC_ECC_ISR_MBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> +
> +/* Write Only - ECC Interrupt Enable Register */
> +#define XSDFEC_ECC_IER_ADDR (0x30)
> +/* Write Only - ECC Interrupt Disable Register */
> +#define XSDFEC_ECC_IDR_ADDR (0x34)
> +/* Read Only - ECC Interrupt Mask Register */
> +#define XSDFEC_ECC_IMR_ADDR (0x38)
> +
> +/* BYPASS Register */
> +#define XSDFEC_BYPASS_ADDR (0x3C)
> +
> /**
> * struct xsdfec_clks - For managing SD-FEC clocks
> * @core_clk: Main processing clock for core
> @@ -48,6 +129,8 @@ struct xsdfec_clks {
> * struct xsdfec_dev - Driver data for SDFEC
> * @regs: device physical base address
> * @dev: pointer to device struct
> + * @state: State of the SDFEC device
> + * @config: Configuration of the SDFEC device
> * @miscdev: Misc device handle
> * @error_data_lock: Error counter and states spinlock
> * @clks: Clocks managed by the SDFEC driver
> @@ -57,16 +140,220 @@ struct xsdfec_clks {
> struct xsdfec_dev {
> void __iomem *regs;
> struct device *dev;
> + enum xsdfec_state state;
> + struct xsdfec_config config;
> struct miscdevice miscdev;
> /* Spinlock to protect state_updated and stats_updated */
> spinlock_t error_data_lock;
> struct xsdfec_clks clks;
> };
>
> +static inline void xsdfec_regwrite(struct xsdfec_dev *xsdfec, u32 addr,
> + u32 value)
> +{
> + dev_dbg(xsdfec->dev, "Writing 0x%x to offset 0x%x", value, addr);
> + iowrite32(value, xsdfec->regs + addr);
> +}
> +
> +static inline u32 xsdfec_regread(struct xsdfec_dev *xsdfec, u32 addr)
> +{
> + u32 rval;
> +
> + rval = ioread32(xsdfec->regs + addr);
> + dev_dbg(xsdfec->dev, "Read value = 0x%x from offset 0x%x", rval, addr);
> + return rval;
> +}
> +
> +static void update_bool_config_from_reg(struct xsdfec_dev *xsdfec,
> + u32 reg_offset, u32 bit_num,
> + char *config_value)
> +{
> + u32 reg_val;
> + u32 bit_mask = 1 << bit_num;
> +
> + reg_val = xsdfec_regread(xsdfec, reg_offset);
> + *config_value = (reg_val & bit_mask) > 0;
> +}
> +
> +static void update_config_from_hw(struct xsdfec_dev *xsdfec)
> +{
> + u32 reg_value;
> + bool sdfec_started;
> +
> + /* Update the Order */
> + reg_value = xsdfec_regread(xsdfec, XSDFEC_ORDER_ADDR);
> + xsdfec->config.order = reg_value;
> +
> + update_bool_config_from_reg(xsdfec, XSDFEC_BYPASS_ADDR,
> + 0, /* Bit Number, maybe change to mask */
> + &xsdfec->config.bypass);
> +
> + update_bool_config_from_reg(xsdfec, XSDFEC_CODE_WR_PROTECT_ADDR,
> + 0, /* Bit Number */
> + &xsdfec->config.code_wr_protect);
> +
> + reg_value = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
> + xsdfec->config.irq.enable_isr = (reg_value & XSDFEC_ISR_MASK) > 0;
> +
> + reg_value = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
> + xsdfec->config.irq.enable_ecc_isr =
> + (reg_value & XSDFEC_ECC_ISR_MASK) > 0;
> +
> + reg_value = xsdfec_regread(xsdfec, XSDFEC_AXIS_ENABLE_ADDR);
> + sdfec_started = (reg_value & XSDFEC_AXIS_IN_ENABLE_MASK) > 0;
> + if (sdfec_started)
> + xsdfec->state = XSDFEC_STARTED;
> + else
> + xsdfec->state = XSDFEC_STOPPED;
> +}
> +
> +static u32
> +xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
> +{
> + u32 axis_width_field = 0;
> +
> + switch (axis_width_cfg) {
> + case XSDFEC_1x128b:
> + axis_width_field = 0;
> + break;
> + case XSDFEC_2x128b:
> + axis_width_field = 1;
> + break;
> + case XSDFEC_4x128b:
> + axis_width_field = 2;
> + break;
> + }
> +
> + return axis_width_field;
> +}
> +
> +static u32 xsdfec_translate_axis_words_cfg_val(enum xsdfec_axis_word_include
> + axis_word_inc_cfg)
> +{
> + u32 axis_words_field = 0;
> +
> + if (axis_word_inc_cfg == XSDFEC_FIXED_VALUE ||
> + axis_word_inc_cfg == XSDFEC_IN_BLOCK)
> + axis_words_field = 0;
> + else if (axis_word_inc_cfg == XSDFEC_PER_AXI_TRANSACTION)
> + axis_words_field = 1;
> +
> + return axis_words_field;
> +}
> +
> +static int xsdfec_cfg_axi_streams(struct xsdfec_dev *xsdfec)
> +{
> + u32 reg_value;
> + u32 dout_words_field;
> + u32 dout_width_field;
> + u32 din_words_field;
> + u32 din_width_field;
> + struct xsdfec_config *config = &xsdfec->config;
> +
> + /* translate config info to register values */
> + dout_words_field =
> + xsdfec_translate_axis_words_cfg_val(config->dout_word_include);
> + dout_width_field =
> + xsdfec_translate_axis_width_cfg_val(config->dout_width);
> + din_words_field =
> + xsdfec_translate_axis_words_cfg_val(config->din_word_include);
> + din_width_field =
> + xsdfec_translate_axis_width_cfg_val(config->din_width);
> +
> + reg_value = dout_words_field << XSDFEC_AXIS_DOUT_WORDS_LSB;
> + reg_value |= dout_width_field << XSDFEC_AXIS_DOUT_WIDTH_LSB;
> + reg_value |= din_words_field << XSDFEC_AXIS_DIN_WORDS_LSB;
> + reg_value |= din_width_field << XSDFEC_AXIS_DIN_WIDTH_LSB;
> +
> + xsdfec_regwrite(xsdfec, XSDFEC_AXIS_WIDTH_ADDR, reg_value);
> +
> + return 0;
> +}
> +
> static const struct file_operations xsdfec_fops = {
> .owner = THIS_MODULE,
> };
>
> +static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
> +{
> + struct device *dev = xsdfec->dev;
> + struct device_node *node = dev->of_node;
> + int rval;
> + const char *fec_code;
> + u32 din_width;
> + u32 din_word_include;
> + u32 dout_width;
> + u32 dout_word_include;
> +
> + rval = of_property_read_string(node, "xlnx,sdfec-code", &fec_code);
> + if (rval < 0)
> + return rval;
> +
> + if (!strcasecmp(fec_code, "ldpc"))
> + xsdfec->config.code = XSDFEC_LDPC_CODE;
> + else if (!strcasecmp(fec_code, "turbo"))
> + xsdfec->config.code = XSDFEC_TURBO_CODE;
> + else
> + return -EINVAL;
> +
> + rval = of_property_read_u32(node, "xlnx,sdfec-din-words",
> + &din_word_include);
> + if (rval < 0)
> + return rval;
> +
> + if (din_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> + xsdfec->config.din_word_include = din_word_include;
> + else
> + return -EINVAL;
> +
> + rval = of_property_read_u32(node, "xlnx,sdfec-din-width", &din_width);
> + if (rval < 0)
> + return rval;
> +
> + switch (din_width) {
> + /* Fall through and set for valid values */
> + case XSDFEC_1x128b:
> + case XSDFEC_2x128b:
> + case XSDFEC_4x128b:
> + xsdfec->config.din_width = din_width;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + rval = of_property_read_u32(node, "xlnx,sdfec-dout-words",
> + &dout_word_include);
> + if (rval < 0)
> + return rval;
> +
> + if (dout_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> + xsdfec->config.dout_word_include = dout_word_include;
> + else
> + return -EINVAL;
> +
> + rval = of_property_read_u32(node, "xlnx,sdfec-dout-width", &dout_width);
> + if (rval < 0)
> + return rval;
> +
> + switch (dout_width) {
> + /* Fall through and set for valid values */
> + case XSDFEC_1x128b:
> + case XSDFEC_2x128b:
> + case XSDFEC_4x128b:
> + xsdfec->config.dout_width = dout_width;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Write LDPC to CODE Register */
> + xsdfec_regwrite(xsdfec, XSDFEC_FEC_CODE_ADDR, xsdfec->config.code);
> +
> + xsdfec_cfg_axi_streams(xsdfec);
> +
> + return 0;
> +}
> +
> static int xsdfec_clk_init(struct platform_device *pdev,
> struct xsdfec_clks *clks)
> {
> @@ -247,6 +534,12 @@ static int xsdfec_probe(struct platform_device *pdev)
> goto err_xsdfec_dev;
> }
>
> + err = xsdfec_parse_of(xsdfec);
> + if (err < 0)
> + goto err_xsdfec_dev;
> +
> + update_config_from_hw(xsdfec);
> +
> /* Save driver private data */
> platform_set_drvdata(pdev, xsdfec);
>
> diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
> new file mode 100644
> index 0000000..7b47a4c
> --- /dev/null
> +++ b/include/uapi/misc/xilinx_sdfec.h
> @@ -0,0 +1,138 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * Xilinx SD-FEC
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Description:
> + * This driver is developed for SDFEC16 IP. It provides a char device
> + * in sysfs and supports file operations like open(), close() and ioctl().
> + */
> +#ifndef __XILINX_SDFEC_H__
> +#define __XILINX_SDFEC_H__
> +
> +#include <linux/types.h>
> +
> +/**
> + * enum xsdfec_code - Code Type.
> + * @XSDFEC_TURBO_CODE: Driver is configured for Turbo mode.
> + * @XSDFEC_LDPC_CODE: Driver is configured for LDPC mode.
> + *
> + * This enum is used to indicate the mode of the driver. The mode is determined
> + * by checking which codes are set in the driver. Note that the mode cannot be
> + * changed by the driver.
> + */
> +enum xsdfec_code {
> + XSDFEC_TURBO_CODE = 0,
> + XSDFEC_LDPC_CODE,
> +};
> +
> +/**
> + * enum xsdfec_order - Order
> + * @XSDFEC_MAINTAIN_ORDER: Maintain order execution of blocks.
> + * @XSDFEC_OUT_OF_ORDER: Out-of-order execution of blocks.
> + *
> + * This enum is used to indicate whether the order of blocks can change from
> + * input to output.
> + */
> +enum xsdfec_order {
> + XSDFEC_MAINTAIN_ORDER = 0,
> + XSDFEC_OUT_OF_ORDER,
> +};
> +
> +/**
> + * enum xsdfec_state - State.
> + * @XSDFEC_INIT: Driver is initialized.
> + * @XSDFEC_STARTED: Driver is started.
> + * @XSDFEC_STOPPED: Driver is stopped.
> + * @XSDFEC_NEEDS_RESET: Driver needs to be reset.
> + * @XSDFEC_PL_RECONFIGURE: Programmable Logic needs to be recofigured.
> + *
> + * This enum is used to indicate the state of the driver.
> + */
> +enum xsdfec_state {
> + XSDFEC_INIT = 0,
> + XSDFEC_STARTED,
> + XSDFEC_STOPPED,
> + XSDFEC_NEEDS_RESET,
> + XSDFEC_PL_RECONFIGURE,
> +};
> +
> +/**
> + * enum xsdfec_axis_width - AXIS_WIDTH.DIN Setting for 128-bit width.
> + * @XSDFEC_1x128b: DIN data input stream consists of a 128-bit lane
> + * @XSDFEC_2x128b: DIN data input stream consists of two 128-bit lanes
> + * @XSDFEC_4x128b: DIN data input stream consists of four 128-bit lanes
> + *
> + * This enum is used to indicate the AXIS_WIDTH.DIN setting for 128-bit width.
> + * The number of lanes of the DIN data input stream depends upon the
> + * AXIS_WIDTH.DIN parameter.
> + */
> +enum xsdfec_axis_width {
> + XSDFEC_1x128b = 1,
> + XSDFEC_2x128b = 2,
> + XSDFEC_4x128b = 4,
> +};
> +
> +/**
> + * enum xsdfec_axis_word_include - Words Configuration.
> + * @XSDFEC_FIXED_VALUE: Fixed, the DIN_WORDS AXI4-Stream interface is removed
> + * from the IP instance and is driven with the specified
> + * number of words.
> + * @XSDFEC_IN_BLOCK: In Block, configures the IP instance to expect a single
> + * DIN_WORDS value per input code block. The DIN_WORDS
> + * interface is present.
> + * @XSDFEC_PER_AXI_TRANSACTION: Per Transaction, configures the IP instance to
> + * expect one DIN_WORDS value per input transaction on the DIN interface. The
> + * DIN_WORDS interface is present.
> + * @XSDFEC_AXIS_WORDS_INCLUDE_MAX: Used to indicate out of bound Words
> + * Configurations.
> + *
> + * This enum is used to specify the DIN_WORDS configuration.
> + */
> +enum xsdfec_axis_word_include {
> + XSDFEC_FIXED_VALUE = 0,
> + XSDFEC_IN_BLOCK,
> + XSDFEC_PER_AXI_TRANSACTION,
> + XSDFEC_AXIS_WORDS_INCLUDE_MAX,
> +};
> +
> +/**
> + * struct xsdfec_irq - Enabling or Disabling Interrupts.
> + * @enable_isr: If true enables the ISR
> + * @enable_ecc_isr: If true enables the ECC ISR
> + */
> +struct xsdfec_irq {
> + __s8 enable_isr;
> + __s8 enable_ecc_isr;
> +};
> +
> +/**
> + * struct xsdfec_config - Configuration of SD-FEC core.
> + * @code: The codes being used by the SD-FEC instance
> + * @order: Order of Operation
> + * @bypass: Is the core being bypassed
> + * @code_wr_protect: Is write protection of LDPC codes enabled
> + * @din_width: Width of the DIN AXI4-Stream
> + * @din_word_include: How DIN_WORDS are inputted
> + * @dout_width: Width of the DOUT AXI4-Stream
> + * @dout_word_include: HOW DOUT_WORDS are outputted
> + * @irq: Enabling or disabling interrupts
> + */
> +struct xsdfec_config {
> + enum xsdfec_code code;
> + enum xsdfec_order order;
> + __s8 bypass;
> + __s8 code_wr_protect;
> + enum xsdfec_axis_width din_width;
> + enum xsdfec_axis_word_include din_word_include;
> + enum xsdfec_axis_width dout_width;
> + enum xsdfec_axis_word_include dout_word_include;

You can't put an 'enum' in a structure that crosses the user/kernel
boundry, as you really do not "know" what the size is going to be. A
compiler can pick whatever width it wants to here.

So, if you "know" this is going to fit in 8 bits, then use __u8 for
these and then cast on the kernel side if you care about trying to keep
typesafe things. If it is going to be bigger then 8 bits, then use the
correct variable.

thanks,

greg k-h

2019-06-09 19:06:37

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Sunday 9 June 2019 12:28
> To: Dragan Cvetic <[email protected]>
> Cc: [email protected]; Michal Simek <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Derek Kiernan <[email protected]>
> Subject: Re: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state
>
> On Sun, Jun 09, 2019 at 01:04:09AM +0100, Dragan Cvetic wrote:
> > Stores configuration based on parameters from the DT
> > node and values from the SD-FEC core plus reads
> > the default state from the SD-FEC core. To obtain
> > values from the core register read, write capabilities
> > have been added plus related register map details.
> >
> > Tested-by: Dragan Cvetic <[email protected]>
> > Signed-off-by: Derek Kiernan <[email protected]>
> > Signed-off-by: Dragan Cvetic <[email protected]>
> > ---
> > drivers/misc/xilinx_sdfec.c | 293 +++++++++++++++++++++++++++++++++++++++
> > include/uapi/misc/xilinx_sdfec.h | 138 ++++++++++++++++++
> > 2 files changed, 431 insertions(+)
> > create mode 100644 include/uapi/misc/xilinx_sdfec.h
> >
> > diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> > index 4524677..ddfca54 100644
> > --- a/drivers/misc/xilinx_sdfec.c
> > +++ b/drivers/misc/xilinx_sdfec.c
> > @@ -20,8 +20,89 @@
> > #include <linux/slab.h>
> > #include <linux/clk.h>
> >
> > +#include <uapi/misc/xilinx_sdfec.h>
> > +
> > static int xsdfec_ndevs;
> >
> > +/* Xilinx SDFEC Register Map */
> > +/* CODE_WRI_PROTECT Register */
> > +#define XSDFEC_CODE_WR_PROTECT_ADDR (0x4)
> > +
> > +/* ACTIVE Register */
> > +#define XSDFEC_ACTIVE_ADDR (0x8)
> > +#define XSDFEC_IS_ACTIVITY_SET (0x1)
> > +
> > +/* AXIS_WIDTH Register */
> > +#define XSDFEC_AXIS_WIDTH_ADDR (0xC)
> > +#define XSDFEC_AXIS_DOUT_WORDS_LSB (5)
> > +#define XSDFEC_AXIS_DOUT_WIDTH_LSB (3)
> > +#define XSDFEC_AXIS_DIN_WORDS_LSB (2)
> > +#define XSDFEC_AXIS_DIN_WIDTH_LSB (0)
> > +
> > +/* AXIS_ENABLE Register */
> > +#define XSDFEC_AXIS_ENABLE_ADDR (0x10)
> > +#define XSDFEC_AXIS_OUT_ENABLE_MASK (0x38)
> > +#define XSDFEC_AXIS_IN_ENABLE_MASK (0x7)
> > +#define XSDFEC_AXIS_ENABLE_MASK \
> > + (XSDFEC_AXIS_OUT_ENABLE_MASK | XSDFEC_AXIS_IN_ENABLE_MASK)
> > +
> > +/* FEC_CODE Register */
> > +#define XSDFEC_FEC_CODE_ADDR (0x14)
> > +
> > +/* ORDER Register Map */
> > +#define XSDFEC_ORDER_ADDR (0x18)
> > +
> > +/* Interrupt Status Register */
> > +#define XSDFEC_ISR_ADDR (0x1C)
> > +/* Interrupt Status Register Bit Mask */
> > +#define XSDFEC_ISR_MASK (0x3F)
> > +
> > +/* Write Only - Interrupt Enable Register */
> > +#define XSDFEC_IER_ADDR (0x20)
> > +/* Write Only - Interrupt Disable Register */
> > +#define XSDFEC_IDR_ADDR (0x24)
> > +/* Read Only - Interrupt Mask Register */
> > +#define XSDFEC_IMR_ADDR (0x28)
> > +
> > +/* ECC Interrupt Status Register */
> > +#define XSDFEC_ECC_ISR_ADDR (0x2C)
> > +/* Single Bit Errors */
> > +#define XSDFEC_ECC_ISR_SBE_MASK (0x7FF)
> > +/* PL Initialize Single Bit Errors */
> > +#define XSDFEC_PL_INIT_ECC_ISR_SBE_MASK (0x3C00000)
> > +/* Multi Bit Errors */
> > +#define XSDFEC_ECC_ISR_MBE_MASK (0x3FF800)
> > +/* PL Initialize Multi Bit Errors */
> > +#define XSDFEC_PL_INIT_ECC_ISR_MBE_MASK (0x3C000000)
> > +/* Multi Bit Error to Event Shift */
> > +#define XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT (11)
> > +/* PL Initialize Multi Bit Error to Event Shift */
> > +#define XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT (4)
> > +/* ECC Interrupt Status Bit Mask */
> > +#define XSDFEC_ECC_ISR_MASK (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_ECC_ISR_MBE_MASK)
> > +/* ECC Interrupt Status PL Initialize Bit Mask */
> > +#define XSDFEC_PL_INIT_ECC_ISR_MASK \
> > + (XSDFEC_PL_INIT_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> > +/* ECC Interrupt Status All Bit Mask */
> > +#define XSDFEC_ALL_ECC_ISR_MASK \
> > + (XSDFEC_ECC_ISR_MASK | XSDFEC_PL_INIT_ECC_ISR_MASK)
> > +/* ECC Interrupt Status Single Bit Errors Mask */
> > +#define XSDFEC_ALL_ECC_ISR_SBE_MASK \
> > + (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_SBE_MASK)
> > +/* ECC Interrupt Status Multi Bit Errors Mask */
> > +#define XSDFEC_ALL_ECC_ISR_MBE_MASK \
> > + (XSDFEC_ECC_ISR_MBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> > +
> > +/* Write Only - ECC Interrupt Enable Register */
> > +#define XSDFEC_ECC_IER_ADDR (0x30)
> > +/* Write Only - ECC Interrupt Disable Register */
> > +#define XSDFEC_ECC_IDR_ADDR (0x34)
> > +/* Read Only - ECC Interrupt Mask Register */
> > +#define XSDFEC_ECC_IMR_ADDR (0x38)
> > +
> > +/* BYPASS Register */
> > +#define XSDFEC_BYPASS_ADDR (0x3C)
> > +
> > /**
> > * struct xsdfec_clks - For managing SD-FEC clocks
> > * @core_clk: Main processing clock for core
> > @@ -48,6 +129,8 @@ struct xsdfec_clks {
> > * struct xsdfec_dev - Driver data for SDFEC
> > * @regs: device physical base address
> > * @dev: pointer to device struct
> > + * @state: State of the SDFEC device
> > + * @config: Configuration of the SDFEC device
> > * @miscdev: Misc device handle
> > * @error_data_lock: Error counter and states spinlock
> > * @clks: Clocks managed by the SDFEC driver
> > @@ -57,16 +140,220 @@ struct xsdfec_clks {
> > struct xsdfec_dev {
> > void __iomem *regs;
> > struct device *dev;
> > + enum xsdfec_state state;
> > + struct xsdfec_config config;
> > struct miscdevice miscdev;
> > /* Spinlock to protect state_updated and stats_updated */
> > spinlock_t error_data_lock;
> > struct xsdfec_clks clks;
> > };
> >
> > +static inline void xsdfec_regwrite(struct xsdfec_dev *xsdfec, u32 addr,
> > + u32 value)
> > +{
> > + dev_dbg(xsdfec->dev, "Writing 0x%x to offset 0x%x", value, addr);
> > + iowrite32(value, xsdfec->regs + addr);
> > +}
> > +
> > +static inline u32 xsdfec_regread(struct xsdfec_dev *xsdfec, u32 addr)
> > +{
> > + u32 rval;
> > +
> > + rval = ioread32(xsdfec->regs + addr);
> > + dev_dbg(xsdfec->dev, "Read value = 0x%x from offset 0x%x", rval, addr);
> > + return rval;
> > +}
> > +
> > +static void update_bool_config_from_reg(struct xsdfec_dev *xsdfec,
> > + u32 reg_offset, u32 bit_num,
> > + char *config_value)
> > +{
> > + u32 reg_val;
> > + u32 bit_mask = 1 << bit_num;
> > +
> > + reg_val = xsdfec_regread(xsdfec, reg_offset);
> > + *config_value = (reg_val & bit_mask) > 0;
> > +}
> > +
> > +static void update_config_from_hw(struct xsdfec_dev *xsdfec)
> > +{
> > + u32 reg_value;
> > + bool sdfec_started;
> > +
> > + /* Update the Order */
> > + reg_value = xsdfec_regread(xsdfec, XSDFEC_ORDER_ADDR);
> > + xsdfec->config.order = reg_value;
> > +
> > + update_bool_config_from_reg(xsdfec, XSDFEC_BYPASS_ADDR,
> > + 0, /* Bit Number, maybe change to mask */
> > + &xsdfec->config.bypass);
> > +
> > + update_bool_config_from_reg(xsdfec, XSDFEC_CODE_WR_PROTECT_ADDR,
> > + 0, /* Bit Number */
> > + &xsdfec->config.code_wr_protect);
> > +
> > + reg_value = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
> > + xsdfec->config.irq.enable_isr = (reg_value & XSDFEC_ISR_MASK) > 0;
> > +
> > + reg_value = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
> > + xsdfec->config.irq.enable_ecc_isr =
> > + (reg_value & XSDFEC_ECC_ISR_MASK) > 0;
> > +
> > + reg_value = xsdfec_regread(xsdfec, XSDFEC_AXIS_ENABLE_ADDR);
> > + sdfec_started = (reg_value & XSDFEC_AXIS_IN_ENABLE_MASK) > 0;
> > + if (sdfec_started)
> > + xsdfec->state = XSDFEC_STARTED;
> > + else
> > + xsdfec->state = XSDFEC_STOPPED;
> > +}
> > +
> > +static u32
> > +xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
> > +{
> > + u32 axis_width_field = 0;
> > +
> > + switch (axis_width_cfg) {
> > + case XSDFEC_1x128b:
> > + axis_width_field = 0;
> > + break;
> > + case XSDFEC_2x128b:
> > + axis_width_field = 1;
> > + break;
> > + case XSDFEC_4x128b:
> > + axis_width_field = 2;
> > + break;
> > + }
> > +
> > + return axis_width_field;
> > +}
> > +
> > +static u32 xsdfec_translate_axis_words_cfg_val(enum xsdfec_axis_word_include
> > + axis_word_inc_cfg)
> > +{
> > + u32 axis_words_field = 0;
> > +
> > + if (axis_word_inc_cfg == XSDFEC_FIXED_VALUE ||
> > + axis_word_inc_cfg == XSDFEC_IN_BLOCK)
> > + axis_words_field = 0;
> > + else if (axis_word_inc_cfg == XSDFEC_PER_AXI_TRANSACTION)
> > + axis_words_field = 1;
> > +
> > + return axis_words_field;
> > +}
> > +
> > +static int xsdfec_cfg_axi_streams(struct xsdfec_dev *xsdfec)
> > +{
> > + u32 reg_value;
> > + u32 dout_words_field;
> > + u32 dout_width_field;
> > + u32 din_words_field;
> > + u32 din_width_field;
> > + struct xsdfec_config *config = &xsdfec->config;
> > +
> > + /* translate config info to register values */
> > + dout_words_field =
> > + xsdfec_translate_axis_words_cfg_val(config->dout_word_include);
> > + dout_width_field =
> > + xsdfec_translate_axis_width_cfg_val(config->dout_width);
> > + din_words_field =
> > + xsdfec_translate_axis_words_cfg_val(config->din_word_include);
> > + din_width_field =
> > + xsdfec_translate_axis_width_cfg_val(config->din_width);
> > +
> > + reg_value = dout_words_field << XSDFEC_AXIS_DOUT_WORDS_LSB;
> > + reg_value |= dout_width_field << XSDFEC_AXIS_DOUT_WIDTH_LSB;
> > + reg_value |= din_words_field << XSDFEC_AXIS_DIN_WORDS_LSB;
> > + reg_value |= din_width_field << XSDFEC_AXIS_DIN_WIDTH_LSB;
> > +
> > + xsdfec_regwrite(xsdfec, XSDFEC_AXIS_WIDTH_ADDR, reg_value);
> > +
> > + return 0;
> > +}
> > +
> > static const struct file_operations xsdfec_fops = {
> > .owner = THIS_MODULE,
> > };
> >
> > +static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
> > +{
> > + struct device *dev = xsdfec->dev;
> > + struct device_node *node = dev->of_node;
> > + int rval;
> > + const char *fec_code;
> > + u32 din_width;
> > + u32 din_word_include;
> > + u32 dout_width;
> > + u32 dout_word_include;
> > +
> > + rval = of_property_read_string(node, "xlnx,sdfec-code", &fec_code);
> > + if (rval < 0)
> > + return rval;
> > +
> > + if (!strcasecmp(fec_code, "ldpc"))
> > + xsdfec->config.code = XSDFEC_LDPC_CODE;
> > + else if (!strcasecmp(fec_code, "turbo"))
> > + xsdfec->config.code = XSDFEC_TURBO_CODE;
> > + else
> > + return -EINVAL;
> > +
> > + rval = of_property_read_u32(node, "xlnx,sdfec-din-words",
> > + &din_word_include);
> > + if (rval < 0)
> > + return rval;
> > +
> > + if (din_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> > + xsdfec->config.din_word_include = din_word_include;
> > + else
> > + return -EINVAL;
> > +
> > + rval = of_property_read_u32(node, "xlnx,sdfec-din-width", &din_width);
> > + if (rval < 0)
> > + return rval;
> > +
> > + switch (din_width) {
> > + /* Fall through and set for valid values */
> > + case XSDFEC_1x128b:
> > + case XSDFEC_2x128b:
> > + case XSDFEC_4x128b:
> > + xsdfec->config.din_width = din_width;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + rval = of_property_read_u32(node, "xlnx,sdfec-dout-words",
> > + &dout_word_include);
> > + if (rval < 0)
> > + return rval;
> > +
> > + if (dout_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> > + xsdfec->config.dout_word_include = dout_word_include;
> > + else
> > + return -EINVAL;
> > +
> > + rval = of_property_read_u32(node, "xlnx,sdfec-dout-width", &dout_width);
> > + if (rval < 0)
> > + return rval;
> > +
> > + switch (dout_width) {
> > + /* Fall through and set for valid values */
> > + case XSDFEC_1x128b:
> > + case XSDFEC_2x128b:
> > + case XSDFEC_4x128b:
> > + xsdfec->config.dout_width = dout_width;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + /* Write LDPC to CODE Register */
> > + xsdfec_regwrite(xsdfec, XSDFEC_FEC_CODE_ADDR, xsdfec->config.code);
> > +
> > + xsdfec_cfg_axi_streams(xsdfec);
> > +
> > + return 0;
> > +}
> > +
> > static int xsdfec_clk_init(struct platform_device *pdev,
> > struct xsdfec_clks *clks)
> > {
> > @@ -247,6 +534,12 @@ static int xsdfec_probe(struct platform_device *pdev)
> > goto err_xsdfec_dev;
> > }
> >
> > + err = xsdfec_parse_of(xsdfec);
> > + if (err < 0)
> > + goto err_xsdfec_dev;
> > +
> > + update_config_from_hw(xsdfec);
> > +
> > /* Save driver private data */
> > platform_set_drvdata(pdev, xsdfec);
> >
> > diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
> > new file mode 100644
> > index 0000000..7b47a4c
> > --- /dev/null
> > +++ b/include/uapi/misc/xilinx_sdfec.h
> > @@ -0,0 +1,138 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/*
> > + * Xilinx SD-FEC
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Description:
> > + * This driver is developed for SDFEC16 IP. It provides a char device
> > + * in sysfs and supports file operations like open(), close() and ioctl().
> > + */
> > +#ifndef __XILINX_SDFEC_H__
> > +#define __XILINX_SDFEC_H__
> > +
> > +#include <linux/types.h>
> > +
> > +/**
> > + * enum xsdfec_code - Code Type.
> > + * @XSDFEC_TURBO_CODE: Driver is configured for Turbo mode.
> > + * @XSDFEC_LDPC_CODE: Driver is configured for LDPC mode.
> > + *
> > + * This enum is used to indicate the mode of the driver. The mode is determined
> > + * by checking which codes are set in the driver. Note that the mode cannot be
> > + * changed by the driver.
> > + */
> > +enum xsdfec_code {
> > + XSDFEC_TURBO_CODE = 0,
> > + XSDFEC_LDPC_CODE,
> > +};
> > +
> > +/**
> > + * enum xsdfec_order - Order
> > + * @XSDFEC_MAINTAIN_ORDER: Maintain order execution of blocks.
> > + * @XSDFEC_OUT_OF_ORDER: Out-of-order execution of blocks.
> > + *
> > + * This enum is used to indicate whether the order of blocks can change from
> > + * input to output.
> > + */
> > +enum xsdfec_order {
> > + XSDFEC_MAINTAIN_ORDER = 0,
> > + XSDFEC_OUT_OF_ORDER,
> > +};
> > +
> > +/**
> > + * enum xsdfec_state - State.
> > + * @XSDFEC_INIT: Driver is initialized.
> > + * @XSDFEC_STARTED: Driver is started.
> > + * @XSDFEC_STOPPED: Driver is stopped.
> > + * @XSDFEC_NEEDS_RESET: Driver needs to be reset.
> > + * @XSDFEC_PL_RECONFIGURE: Programmable Logic needs to be recofigured.
> > + *
> > + * This enum is used to indicate the state of the driver.
> > + */
> > +enum xsdfec_state {
> > + XSDFEC_INIT = 0,
> > + XSDFEC_STARTED,
> > + XSDFEC_STOPPED,
> > + XSDFEC_NEEDS_RESET,
> > + XSDFEC_PL_RECONFIGURE,
> > +};
> > +
> > +/**
> > + * enum xsdfec_axis_width - AXIS_WIDTH.DIN Setting for 128-bit width.
> > + * @XSDFEC_1x128b: DIN data input stream consists of a 128-bit lane
> > + * @XSDFEC_2x128b: DIN data input stream consists of two 128-bit lanes
> > + * @XSDFEC_4x128b: DIN data input stream consists of four 128-bit lanes
> > + *
> > + * This enum is used to indicate the AXIS_WIDTH.DIN setting for 128-bit width.
> > + * The number of lanes of the DIN data input stream depends upon the
> > + * AXIS_WIDTH.DIN parameter.
> > + */
> > +enum xsdfec_axis_width {
> > + XSDFEC_1x128b = 1,
> > + XSDFEC_2x128b = 2,
> > + XSDFEC_4x128b = 4,
> > +};
> > +
> > +/**
> > + * enum xsdfec_axis_word_include - Words Configuration.
> > + * @XSDFEC_FIXED_VALUE: Fixed, the DIN_WORDS AXI4-Stream interface is removed
> > + * from the IP instance and is driven with the specified
> > + * number of words.
> > + * @XSDFEC_IN_BLOCK: In Block, configures the IP instance to expect a single
> > + * DIN_WORDS value per input code block. The DIN_WORDS
> > + * interface is present.
> > + * @XSDFEC_PER_AXI_TRANSACTION: Per Transaction, configures the IP instance to
> > + * expect one DIN_WORDS value per input transaction on the DIN interface. The
> > + * DIN_WORDS interface is present.
> > + * @XSDFEC_AXIS_WORDS_INCLUDE_MAX: Used to indicate out of bound Words
> > + * Configurations.
> > + *
> > + * This enum is used to specify the DIN_WORDS configuration.
> > + */
> > +enum xsdfec_axis_word_include {
> > + XSDFEC_FIXED_VALUE = 0,
> > + XSDFEC_IN_BLOCK,
> > + XSDFEC_PER_AXI_TRANSACTION,
> > + XSDFEC_AXIS_WORDS_INCLUDE_MAX,
> > +};
> > +
> > +/**
> > + * struct xsdfec_irq - Enabling or Disabling Interrupts.
> > + * @enable_isr: If true enables the ISR
> > + * @enable_ecc_isr: If true enables the ECC ISR
> > + */
> > +struct xsdfec_irq {
> > + __s8 enable_isr;
> > + __s8 enable_ecc_isr;
> > +};
> > +
> > +/**
> > + * struct xsdfec_config - Configuration of SD-FEC core.
> > + * @code: The codes being used by the SD-FEC instance
> > + * @order: Order of Operation
> > + * @bypass: Is the core being bypassed
> > + * @code_wr_protect: Is write protection of LDPC codes enabled
> > + * @din_width: Width of the DIN AXI4-Stream
> > + * @din_word_include: How DIN_WORDS are inputted
> > + * @dout_width: Width of the DOUT AXI4-Stream
> > + * @dout_word_include: HOW DOUT_WORDS are outputted
> > + * @irq: Enabling or disabling interrupts
> > + */
> > +struct xsdfec_config {
> > + enum xsdfec_code code;
> > + enum xsdfec_order order;
> > + __s8 bypass;
> > + __s8 code_wr_protect;
> > + enum xsdfec_axis_width din_width;
> > + enum xsdfec_axis_word_include din_word_include;
> > + enum xsdfec_axis_width dout_width;
> > + enum xsdfec_axis_word_include dout_word_include;
>
> You can't put an 'enum' in a structure that crosses the user/kernel
> boundry, as you really do not "know" what the size is going to be. A
> compiler can pick whatever width it wants to here.
>
> So, if you "know" this is going to fit in 8 bits, then use __u8 for
> these and then cast on the kernel side if you care about trying to keep
> typesafe things. If it is going to be bigger then 8 bits, then use the
> correct variable.
>

Accepted.
I'm planning to keep the enum definitions in include. They are user/kernel interfaces.
Please confirm this is OK with you.

Thanks
Dragan

> thanks,
>
> greg k-h

2019-06-09 21:13:47

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state

On Sun, Jun 09, 2019 at 07:04:05PM +0000, Dragan Cvetic wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: Sunday 9 June 2019 12:28
> > To: Dragan Cvetic <[email protected]>
> > Cc: [email protected]; Michal Simek <[email protected]>; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]; Derek Kiernan <[email protected]>
> > Subject: Re: [PATCH V5 04/11] misc: xilinx_sdfec: Store driver config and state
> >
> > On Sun, Jun 09, 2019 at 01:04:09AM +0100, Dragan Cvetic wrote:
> > > Stores configuration based on parameters from the DT
> > > node and values from the SD-FEC core plus reads
> > > the default state from the SD-FEC core. To obtain
> > > values from the core register read, write capabilities
> > > have been added plus related register map details.
> > >
> > > Tested-by: Dragan Cvetic <[email protected]>
> > > Signed-off-by: Derek Kiernan <[email protected]>
> > > Signed-off-by: Dragan Cvetic <[email protected]>
> > > ---
> > > drivers/misc/xilinx_sdfec.c | 293 +++++++++++++++++++++++++++++++++++++++
> > > include/uapi/misc/xilinx_sdfec.h | 138 ++++++++++++++++++
> > > 2 files changed, 431 insertions(+)
> > > create mode 100644 include/uapi/misc/xilinx_sdfec.h
> > >
> > > diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> > > index 4524677..ddfca54 100644
> > > --- a/drivers/misc/xilinx_sdfec.c
> > > +++ b/drivers/misc/xilinx_sdfec.c
> > > @@ -20,8 +20,89 @@
> > > #include <linux/slab.h>
> > > #include <linux/clk.h>
> > >
> > > +#include <uapi/misc/xilinx_sdfec.h>
> > > +
> > > static int xsdfec_ndevs;
> > >
> > > +/* Xilinx SDFEC Register Map */
> > > +/* CODE_WRI_PROTECT Register */
> > > +#define XSDFEC_CODE_WR_PROTECT_ADDR (0x4)
> > > +
> > > +/* ACTIVE Register */
> > > +#define XSDFEC_ACTIVE_ADDR (0x8)
> > > +#define XSDFEC_IS_ACTIVITY_SET (0x1)
> > > +
> > > +/* AXIS_WIDTH Register */
> > > +#define XSDFEC_AXIS_WIDTH_ADDR (0xC)
> > > +#define XSDFEC_AXIS_DOUT_WORDS_LSB (5)
> > > +#define XSDFEC_AXIS_DOUT_WIDTH_LSB (3)
> > > +#define XSDFEC_AXIS_DIN_WORDS_LSB (2)
> > > +#define XSDFEC_AXIS_DIN_WIDTH_LSB (0)
> > > +
> > > +/* AXIS_ENABLE Register */
> > > +#define XSDFEC_AXIS_ENABLE_ADDR (0x10)
> > > +#define XSDFEC_AXIS_OUT_ENABLE_MASK (0x38)
> > > +#define XSDFEC_AXIS_IN_ENABLE_MASK (0x7)
> > > +#define XSDFEC_AXIS_ENABLE_MASK \
> > > + (XSDFEC_AXIS_OUT_ENABLE_MASK | XSDFEC_AXIS_IN_ENABLE_MASK)
> > > +
> > > +/* FEC_CODE Register */
> > > +#define XSDFEC_FEC_CODE_ADDR (0x14)
> > > +
> > > +/* ORDER Register Map */
> > > +#define XSDFEC_ORDER_ADDR (0x18)
> > > +
> > > +/* Interrupt Status Register */
> > > +#define XSDFEC_ISR_ADDR (0x1C)
> > > +/* Interrupt Status Register Bit Mask */
> > > +#define XSDFEC_ISR_MASK (0x3F)
> > > +
> > > +/* Write Only - Interrupt Enable Register */
> > > +#define XSDFEC_IER_ADDR (0x20)
> > > +/* Write Only - Interrupt Disable Register */
> > > +#define XSDFEC_IDR_ADDR (0x24)
> > > +/* Read Only - Interrupt Mask Register */
> > > +#define XSDFEC_IMR_ADDR (0x28)
> > > +
> > > +/* ECC Interrupt Status Register */
> > > +#define XSDFEC_ECC_ISR_ADDR (0x2C)
> > > +/* Single Bit Errors */
> > > +#define XSDFEC_ECC_ISR_SBE_MASK (0x7FF)
> > > +/* PL Initialize Single Bit Errors */
> > > +#define XSDFEC_PL_INIT_ECC_ISR_SBE_MASK (0x3C00000)
> > > +/* Multi Bit Errors */
> > > +#define XSDFEC_ECC_ISR_MBE_MASK (0x3FF800)
> > > +/* PL Initialize Multi Bit Errors */
> > > +#define XSDFEC_PL_INIT_ECC_ISR_MBE_MASK (0x3C000000)
> > > +/* Multi Bit Error to Event Shift */
> > > +#define XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT (11)
> > > +/* PL Initialize Multi Bit Error to Event Shift */
> > > +#define XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT (4)
> > > +/* ECC Interrupt Status Bit Mask */
> > > +#define XSDFEC_ECC_ISR_MASK (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_ECC_ISR_MBE_MASK)
> > > +/* ECC Interrupt Status PL Initialize Bit Mask */
> > > +#define XSDFEC_PL_INIT_ECC_ISR_MASK \
> > > + (XSDFEC_PL_INIT_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> > > +/* ECC Interrupt Status All Bit Mask */
> > > +#define XSDFEC_ALL_ECC_ISR_MASK \
> > > + (XSDFEC_ECC_ISR_MASK | XSDFEC_PL_INIT_ECC_ISR_MASK)
> > > +/* ECC Interrupt Status Single Bit Errors Mask */
> > > +#define XSDFEC_ALL_ECC_ISR_SBE_MASK \
> > > + (XSDFEC_ECC_ISR_SBE_MASK | XSDFEC_PL_INIT_ECC_ISR_SBE_MASK)
> > > +/* ECC Interrupt Status Multi Bit Errors Mask */
> > > +#define XSDFEC_ALL_ECC_ISR_MBE_MASK \
> > > + (XSDFEC_ECC_ISR_MBE_MASK | XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
> > > +
> > > +/* Write Only - ECC Interrupt Enable Register */
> > > +#define XSDFEC_ECC_IER_ADDR (0x30)
> > > +/* Write Only - ECC Interrupt Disable Register */
> > > +#define XSDFEC_ECC_IDR_ADDR (0x34)
> > > +/* Read Only - ECC Interrupt Mask Register */
> > > +#define XSDFEC_ECC_IMR_ADDR (0x38)
> > > +
> > > +/* BYPASS Register */
> > > +#define XSDFEC_BYPASS_ADDR (0x3C)
> > > +
> > > /**
> > > * struct xsdfec_clks - For managing SD-FEC clocks
> > > * @core_clk: Main processing clock for core
> > > @@ -48,6 +129,8 @@ struct xsdfec_clks {
> > > * struct xsdfec_dev - Driver data for SDFEC
> > > * @regs: device physical base address
> > > * @dev: pointer to device struct
> > > + * @state: State of the SDFEC device
> > > + * @config: Configuration of the SDFEC device
> > > * @miscdev: Misc device handle
> > > * @error_data_lock: Error counter and states spinlock
> > > * @clks: Clocks managed by the SDFEC driver
> > > @@ -57,16 +140,220 @@ struct xsdfec_clks {
> > > struct xsdfec_dev {
> > > void __iomem *regs;
> > > struct device *dev;
> > > + enum xsdfec_state state;
> > > + struct xsdfec_config config;
> > > struct miscdevice miscdev;
> > > /* Spinlock to protect state_updated and stats_updated */
> > > spinlock_t error_data_lock;
> > > struct xsdfec_clks clks;
> > > };
> > >
> > > +static inline void xsdfec_regwrite(struct xsdfec_dev *xsdfec, u32 addr,
> > > + u32 value)
> > > +{
> > > + dev_dbg(xsdfec->dev, "Writing 0x%x to offset 0x%x", value, addr);
> > > + iowrite32(value, xsdfec->regs + addr);
> > > +}
> > > +
> > > +static inline u32 xsdfec_regread(struct xsdfec_dev *xsdfec, u32 addr)
> > > +{
> > > + u32 rval;
> > > +
> > > + rval = ioread32(xsdfec->regs + addr);
> > > + dev_dbg(xsdfec->dev, "Read value = 0x%x from offset 0x%x", rval, addr);
> > > + return rval;
> > > +}
> > > +
> > > +static void update_bool_config_from_reg(struct xsdfec_dev *xsdfec,
> > > + u32 reg_offset, u32 bit_num,
> > > + char *config_value)
> > > +{
> > > + u32 reg_val;
> > > + u32 bit_mask = 1 << bit_num;
> > > +
> > > + reg_val = xsdfec_regread(xsdfec, reg_offset);
> > > + *config_value = (reg_val & bit_mask) > 0;
> > > +}
> > > +
> > > +static void update_config_from_hw(struct xsdfec_dev *xsdfec)
> > > +{
> > > + u32 reg_value;
> > > + bool sdfec_started;
> > > +
> > > + /* Update the Order */
> > > + reg_value = xsdfec_regread(xsdfec, XSDFEC_ORDER_ADDR);
> > > + xsdfec->config.order = reg_value;
> > > +
> > > + update_bool_config_from_reg(xsdfec, XSDFEC_BYPASS_ADDR,
> > > + 0, /* Bit Number, maybe change to mask */
> > > + &xsdfec->config.bypass);
> > > +
> > > + update_bool_config_from_reg(xsdfec, XSDFEC_CODE_WR_PROTECT_ADDR,
> > > + 0, /* Bit Number */
> > > + &xsdfec->config.code_wr_protect);
> > > +
> > > + reg_value = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
> > > + xsdfec->config.irq.enable_isr = (reg_value & XSDFEC_ISR_MASK) > 0;
> > > +
> > > + reg_value = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
> > > + xsdfec->config.irq.enable_ecc_isr =
> > > + (reg_value & XSDFEC_ECC_ISR_MASK) > 0;
> > > +
> > > + reg_value = xsdfec_regread(xsdfec, XSDFEC_AXIS_ENABLE_ADDR);
> > > + sdfec_started = (reg_value & XSDFEC_AXIS_IN_ENABLE_MASK) > 0;
> > > + if (sdfec_started)
> > > + xsdfec->state = XSDFEC_STARTED;
> > > + else
> > > + xsdfec->state = XSDFEC_STOPPED;
> > > +}
> > > +
> > > +static u32
> > > +xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
> > > +{
> > > + u32 axis_width_field = 0;
> > > +
> > > + switch (axis_width_cfg) {
> > > + case XSDFEC_1x128b:
> > > + axis_width_field = 0;
> > > + break;
> > > + case XSDFEC_2x128b:
> > > + axis_width_field = 1;
> > > + break;
> > > + case XSDFEC_4x128b:
> > > + axis_width_field = 2;
> > > + break;
> > > + }
> > > +
> > > + return axis_width_field;
> > > +}
> > > +
> > > +static u32 xsdfec_translate_axis_words_cfg_val(enum xsdfec_axis_word_include
> > > + axis_word_inc_cfg)
> > > +{
> > > + u32 axis_words_field = 0;
> > > +
> > > + if (axis_word_inc_cfg == XSDFEC_FIXED_VALUE ||
> > > + axis_word_inc_cfg == XSDFEC_IN_BLOCK)
> > > + axis_words_field = 0;
> > > + else if (axis_word_inc_cfg == XSDFEC_PER_AXI_TRANSACTION)
> > > + axis_words_field = 1;
> > > +
> > > + return axis_words_field;
> > > +}
> > > +
> > > +static int xsdfec_cfg_axi_streams(struct xsdfec_dev *xsdfec)
> > > +{
> > > + u32 reg_value;
> > > + u32 dout_words_field;
> > > + u32 dout_width_field;
> > > + u32 din_words_field;
> > > + u32 din_width_field;
> > > + struct xsdfec_config *config = &xsdfec->config;
> > > +
> > > + /* translate config info to register values */
> > > + dout_words_field =
> > > + xsdfec_translate_axis_words_cfg_val(config->dout_word_include);
> > > + dout_width_field =
> > > + xsdfec_translate_axis_width_cfg_val(config->dout_width);
> > > + din_words_field =
> > > + xsdfec_translate_axis_words_cfg_val(config->din_word_include);
> > > + din_width_field =
> > > + xsdfec_translate_axis_width_cfg_val(config->din_width);
> > > +
> > > + reg_value = dout_words_field << XSDFEC_AXIS_DOUT_WORDS_LSB;
> > > + reg_value |= dout_width_field << XSDFEC_AXIS_DOUT_WIDTH_LSB;
> > > + reg_value |= din_words_field << XSDFEC_AXIS_DIN_WORDS_LSB;
> > > + reg_value |= din_width_field << XSDFEC_AXIS_DIN_WIDTH_LSB;
> > > +
> > > + xsdfec_regwrite(xsdfec, XSDFEC_AXIS_WIDTH_ADDR, reg_value);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct file_operations xsdfec_fops = {
> > > .owner = THIS_MODULE,
> > > };
> > >
> > > +static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
> > > +{
> > > + struct device *dev = xsdfec->dev;
> > > + struct device_node *node = dev->of_node;
> > > + int rval;
> > > + const char *fec_code;
> > > + u32 din_width;
> > > + u32 din_word_include;
> > > + u32 dout_width;
> > > + u32 dout_word_include;
> > > +
> > > + rval = of_property_read_string(node, "xlnx,sdfec-code", &fec_code);
> > > + if (rval < 0)
> > > + return rval;
> > > +
> > > + if (!strcasecmp(fec_code, "ldpc"))
> > > + xsdfec->config.code = XSDFEC_LDPC_CODE;
> > > + else if (!strcasecmp(fec_code, "turbo"))
> > > + xsdfec->config.code = XSDFEC_TURBO_CODE;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + rval = of_property_read_u32(node, "xlnx,sdfec-din-words",
> > > + &din_word_include);
> > > + if (rval < 0)
> > > + return rval;
> > > +
> > > + if (din_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> > > + xsdfec->config.din_word_include = din_word_include;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + rval = of_property_read_u32(node, "xlnx,sdfec-din-width", &din_width);
> > > + if (rval < 0)
> > > + return rval;
> > > +
> > > + switch (din_width) {
> > > + /* Fall through and set for valid values */
> > > + case XSDFEC_1x128b:
> > > + case XSDFEC_2x128b:
> > > + case XSDFEC_4x128b:
> > > + xsdfec->config.din_width = din_width;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + rval = of_property_read_u32(node, "xlnx,sdfec-dout-words",
> > > + &dout_word_include);
> > > + if (rval < 0)
> > > + return rval;
> > > +
> > > + if (dout_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX)
> > > + xsdfec->config.dout_word_include = dout_word_include;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + rval = of_property_read_u32(node, "xlnx,sdfec-dout-width", &dout_width);
> > > + if (rval < 0)
> > > + return rval;
> > > +
> > > + switch (dout_width) {
> > > + /* Fall through and set for valid values */
> > > + case XSDFEC_1x128b:
> > > + case XSDFEC_2x128b:
> > > + case XSDFEC_4x128b:
> > > + xsdfec->config.dout_width = dout_width;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Write LDPC to CODE Register */
> > > + xsdfec_regwrite(xsdfec, XSDFEC_FEC_CODE_ADDR, xsdfec->config.code);
> > > +
> > > + xsdfec_cfg_axi_streams(xsdfec);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int xsdfec_clk_init(struct platform_device *pdev,
> > > struct xsdfec_clks *clks)
> > > {
> > > @@ -247,6 +534,12 @@ static int xsdfec_probe(struct platform_device *pdev)
> > > goto err_xsdfec_dev;
> > > }
> > >
> > > + err = xsdfec_parse_of(xsdfec);
> > > + if (err < 0)
> > > + goto err_xsdfec_dev;
> > > +
> > > + update_config_from_hw(xsdfec);
> > > +
> > > /* Save driver private data */
> > > platform_set_drvdata(pdev, xsdfec);
> > >
> > > diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
> > > new file mode 100644
> > > index 0000000..7b47a4c
> > > --- /dev/null
> > > +++ b/include/uapi/misc/xilinx_sdfec.h
> > > @@ -0,0 +1,138 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > > +/*
> > > + * Xilinx SD-FEC
> > > + *
> > > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > > + *
> > > + * Description:
> > > + * This driver is developed for SDFEC16 IP. It provides a char device
> > > + * in sysfs and supports file operations like open(), close() and ioctl().
> > > + */
> > > +#ifndef __XILINX_SDFEC_H__
> > > +#define __XILINX_SDFEC_H__
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +/**
> > > + * enum xsdfec_code - Code Type.
> > > + * @XSDFEC_TURBO_CODE: Driver is configured for Turbo mode.
> > > + * @XSDFEC_LDPC_CODE: Driver is configured for LDPC mode.
> > > + *
> > > + * This enum is used to indicate the mode of the driver. The mode is determined
> > > + * by checking which codes are set in the driver. Note that the mode cannot be
> > > + * changed by the driver.
> > > + */
> > > +enum xsdfec_code {
> > > + XSDFEC_TURBO_CODE = 0,
> > > + XSDFEC_LDPC_CODE,
> > > +};
> > > +
> > > +/**
> > > + * enum xsdfec_order - Order
> > > + * @XSDFEC_MAINTAIN_ORDER: Maintain order execution of blocks.
> > > + * @XSDFEC_OUT_OF_ORDER: Out-of-order execution of blocks.
> > > + *
> > > + * This enum is used to indicate whether the order of blocks can change from
> > > + * input to output.
> > > + */
> > > +enum xsdfec_order {
> > > + XSDFEC_MAINTAIN_ORDER = 0,
> > > + XSDFEC_OUT_OF_ORDER,
> > > +};
> > > +
> > > +/**
> > > + * enum xsdfec_state - State.
> > > + * @XSDFEC_INIT: Driver is initialized.
> > > + * @XSDFEC_STARTED: Driver is started.
> > > + * @XSDFEC_STOPPED: Driver is stopped.
> > > + * @XSDFEC_NEEDS_RESET: Driver needs to be reset.
> > > + * @XSDFEC_PL_RECONFIGURE: Programmable Logic needs to be recofigured.
> > > + *
> > > + * This enum is used to indicate the state of the driver.
> > > + */
> > > +enum xsdfec_state {
> > > + XSDFEC_INIT = 0,
> > > + XSDFEC_STARTED,
> > > + XSDFEC_STOPPED,
> > > + XSDFEC_NEEDS_RESET,
> > > + XSDFEC_PL_RECONFIGURE,
> > > +};
> > > +
> > > +/**
> > > + * enum xsdfec_axis_width - AXIS_WIDTH.DIN Setting for 128-bit width.
> > > + * @XSDFEC_1x128b: DIN data input stream consists of a 128-bit lane
> > > + * @XSDFEC_2x128b: DIN data input stream consists of two 128-bit lanes
> > > + * @XSDFEC_4x128b: DIN data input stream consists of four 128-bit lanes
> > > + *
> > > + * This enum is used to indicate the AXIS_WIDTH.DIN setting for 128-bit width.
> > > + * The number of lanes of the DIN data input stream depends upon the
> > > + * AXIS_WIDTH.DIN parameter.
> > > + */
> > > +enum xsdfec_axis_width {
> > > + XSDFEC_1x128b = 1,
> > > + XSDFEC_2x128b = 2,
> > > + XSDFEC_4x128b = 4,
> > > +};
> > > +
> > > +/**
> > > + * enum xsdfec_axis_word_include - Words Configuration.
> > > + * @XSDFEC_FIXED_VALUE: Fixed, the DIN_WORDS AXI4-Stream interface is removed
> > > + * from the IP instance and is driven with the specified
> > > + * number of words.
> > > + * @XSDFEC_IN_BLOCK: In Block, configures the IP instance to expect a single
> > > + * DIN_WORDS value per input code block. The DIN_WORDS
> > > + * interface is present.
> > > + * @XSDFEC_PER_AXI_TRANSACTION: Per Transaction, configures the IP instance to
> > > + * expect one DIN_WORDS value per input transaction on the DIN interface. The
> > > + * DIN_WORDS interface is present.
> > > + * @XSDFEC_AXIS_WORDS_INCLUDE_MAX: Used to indicate out of bound Words
> > > + * Configurations.
> > > + *
> > > + * This enum is used to specify the DIN_WORDS configuration.
> > > + */
> > > +enum xsdfec_axis_word_include {
> > > + XSDFEC_FIXED_VALUE = 0,
> > > + XSDFEC_IN_BLOCK,
> > > + XSDFEC_PER_AXI_TRANSACTION,
> > > + XSDFEC_AXIS_WORDS_INCLUDE_MAX,
> > > +};
> > > +
> > > +/**
> > > + * struct xsdfec_irq - Enabling or Disabling Interrupts.
> > > + * @enable_isr: If true enables the ISR
> > > + * @enable_ecc_isr: If true enables the ECC ISR
> > > + */
> > > +struct xsdfec_irq {
> > > + __s8 enable_isr;
> > > + __s8 enable_ecc_isr;
> > > +};
> > > +
> > > +/**
> > > + * struct xsdfec_config - Configuration of SD-FEC core.
> > > + * @code: The codes being used by the SD-FEC instance
> > > + * @order: Order of Operation
> > > + * @bypass: Is the core being bypassed
> > > + * @code_wr_protect: Is write protection of LDPC codes enabled
> > > + * @din_width: Width of the DIN AXI4-Stream
> > > + * @din_word_include: How DIN_WORDS are inputted
> > > + * @dout_width: Width of the DOUT AXI4-Stream
> > > + * @dout_word_include: HOW DOUT_WORDS are outputted
> > > + * @irq: Enabling or disabling interrupts
> > > + */
> > > +struct xsdfec_config {
> > > + enum xsdfec_code code;
> > > + enum xsdfec_order order;
> > > + __s8 bypass;
> > > + __s8 code_wr_protect;
> > > + enum xsdfec_axis_width din_width;
> > > + enum xsdfec_axis_word_include din_word_include;
> > > + enum xsdfec_axis_width dout_width;
> > > + enum xsdfec_axis_word_include dout_word_include;
> >
> > You can't put an 'enum' in a structure that crosses the user/kernel
> > boundry, as you really do not "know" what the size is going to be. A
> > compiler can pick whatever width it wants to here.
> >
> > So, if you "know" this is going to fit in 8 bits, then use __u8 for
> > these and then cast on the kernel side if you care about trying to keep
> > typesafe things. If it is going to be bigger then 8 bits, then use the
> > correct variable.
> >
>
> Accepted.
> I'm planning to keep the enum definitions in include. They are user/kernel interfaces.
> Please confirm this is OK with you.

definitions where you actually define each of them is fine (hint,
you are not doing that here), just don't make it a variable type in the
structure.

thanks,

greg k-h