2019-03-19 12:05:40

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 00/12] misc: xilinx sd-fec driver

This patchset is adding the full Soft Decision Forward Error
Correction (SD-FEC) driver implementation, driver DT binding and
driver documentation.

Forward Error Correction (FEC) codes such as Low Density Parity
Check (LDPC) and turbo codes provide a means to control errors in
data transmissions over unreliable or noisy communication
channels. The SD-FEC Integrated Block is an optimized block for
soft-decision decoding of these codes. Fixed turbo codes are
supported directly, whereas custom and standardized LDPC codes
are supported through the ability to specify the parity check
matrix through an AXI4-Lite bus or using the optional programmable
(PL)-based support logic. For the further information see
https://www.xilinx.com/support/documentation/ip_documentation/
sd_fec/v1_1/pg256-sdfec-integrated-block.pdf

This driver is a platform device driver which supports SDFEC16
(16nm) IP. SD-FEC driver supports LDPC decoding and encoding and
Turbo code decoding. LDPC codes can be specified on
a codeword-by-codeword basis, also a custom LDPC code can be used.

The SD-FEC driver exposes a char device interface and supports
file operations: open(), close(), poll() and ioctl(). The driver
allows only one usage of the device, open() limits the number of
driver instances. The driver also utilize Common Clock Framework
(CCF).

The control and monitoring is supported over ioctl system call.
The features supported by ioctl():
- enable or disable data pipes to/from device
- configure the FEC algorithm parameters
- set the order of data
- provide a control of a SDFEC bypass option
- activates/deactivates SD-FEC
- collect and provide statistical data
- enable/disable interrupt mode

Poll can be utilized to detect errors on IRQ trigger rather than
using looping status and stats ioctl's.

Reviewed-by: Michal Simek <[email protected]>
Tested-by: Santhosh Dyavanapally <[email protected]>
Tested by: Punnaiah Choudary Kalluri <[email protected]>
Tested-by: Dragan Cvetic <[email protected]>
Tested-by: Derek Kiernan <[email protected]>
Signed-off-by: Derek Kiernan <[email protected]>
Signed-off-by: Dragan Cvetic <[email protected]>

Dragan Cvetic (12):
dt-bindings: xilinx-sdfec: Add SDFEC binding
misc: xilinx-sdfec: add core driver
misc: xilinx_sdfec: Add CCF support
misc: xilinx_sdfec: Add open, close and ioctl
misc: xilinx_sdfec: Store driver config and state
misc: xilinx_sdfec: Add ability to configure turbo mode
misc: xilinx_sdfec: Add ability to configure LDPC
misc: xilinx_sdfec: Add ability to get/set config
misc: xilinx_sdfec: Support poll file operation
misc: xilinx_sdfec: Add stats & status ioctls
Docs: misc: xilinx_sdfec: Add documentation
MAINTAINERS: add maintainer for SD-FEC support

.../devicetree/bindings/misc/xlnx,sd-fec.txt | 58 +
Documentation/misc-devices/index.rst | 1 +
Documentation/misc-devices/xilinx_sdfec.rst | 286 ++++
MAINTAINERS | 12 +
drivers/misc/Kconfig | 12 +
drivers/misc/Makefile | 1 +
drivers/misc/xilinx_sdfec.c | 1683 ++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 470 ++++++
8 files changed, 2523 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
create mode 100644 Documentation/misc-devices/xilinx_sdfec.rst
create mode 100644 drivers/misc/xilinx_sdfec.c
create mode 100644 include/uapi/misc/xilinx_sdfec.h

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.


2019-03-19 12:05:44

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 01/12] dt-bindings: xilinx-sdfec: Add SDFEC binding

Add the Soft Decision Forward Error Correction (SDFEC) Engine
bindings which is available for the Zynq UltraScale+ RFSoC
FPGA's.

Signed-off-by: Dragan Cvetic <[email protected]>
Signed-off-by: Derek Kiernan <[email protected]>
---
.../devicetree/bindings/misc/xlnx,sd-fec.txt | 58 ++++++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt

diff --git a/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
new file mode 100644
index 0000000..8af0adc
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
@@ -0,0 +1,58 @@
+* Xilinx SDFEC(16nm) IP *
+
+The Soft Decision Forward Error Correction (SDFEC) Engine is a Hard IP block
+which provides high-throughput LDPC and Turbo Code implementations.
+The LDPC decode & encode functionality is capable of covering a range of
+customer specified Quasi-cyclic (QC) codes. The Turbo decode functionality
+principally covers codes used by LTE. The FEC Engine offers significant
+power and area savings versus implementations done in the FPGA fabric.
+
+
+Required properties:
+- compatible: Must be "xlnx,sd-fec-1.1"
+- clock-names : List of input clock names from the following:
+ - "core_clk", Main processing clock for processing core (required)
+ - "s_axi_aclk", AXI4-Lite memory-mapped slave interface clock (required)
+ - "s_axis_din_aclk", DIN AXI4-Stream Slave interface clock (optional)
+ - "s_axis_din_words-aclk", DIN_WORDS AXI4-Stream Slave interface clock (optional)
+ - "s_axis_ctrl_aclk", Control input AXI4-Stream Slave interface clock (optional)
+ - "m_axis_dout_aclk", DOUT AXI4-Stream Master interface clock (optional)
+ - "m_axis_dout_words_aclk", DOUT_WORDS AXI4-Stream Master interface clock (optional)
+ - "m_axis_status_aclk", Status output AXI4-Stream Master interface clock (optional)
+- clocks : Clock phandles (see clock_bindings.txt for details).
+- reg: Should contain Xilinx SDFEC 16nm Hardened IP block registers
+ location and length.
+- xlnx,sdfec-code : Should contain "ldpc" or "turbo" to describe the codes
+ being used.
+- xlnx,sdfec-din-words : A value 0 indicates that the DIN_WORDS interface is
+ driven with a fixed value and is not present on the device, a value of 1
+ configures the DIN_WORDS to be block based, while a value of 2 configures the
+ DIN_WORDS input to be supplied for each AXI transaction.
+- xlnx,sdfec-din-width : Configures the DIN AXI stream where a value of 1
+ configures a width of "1x128b", 2 a width of "2x128b" and 4 configures a width
+ of "4x128b".
+- xlnx,sdfec-dout-words : A value 0 indicates that the DOUT_WORDS interface is
+ driven with a fixed value and is not present on the device, a value of 1
+ configures the DOUT_WORDS to be block based, while a value of 2 configures the
+ DOUT_WORDS input to be supplied for each AXI transaction.
+- xlnx,sdfec-dout-width : Configures the DOUT AXI stream where a value of 1
+ configures a width of "1x128b", 2 a width of "2x128b" and 4 configures a width
+ of "4x128b".
+Optional properties:
+- interrupts: should contain SDFEC interrupt number
+
+Example
+---------------------------------------
+ sd_fec_0: sd-fec@a0040000 {
+ compatible = "xlnx,sd-fec-1.1";
+ clock-names = "core_clk", "s_axi_aclk", "s_axis_ctrl_aclk", "s_axis_din_aclk", "m_axis_status_aclk", "m_axis_dout_aclk";
+ clocks = <&misc_clk_2>, <&misc_clk_0>, <&misc_clk_1>, <&misc_clk_1>, <&misc_clk_1>, <&misc_clk_1>;
+ reg = <0x0 0xa0040000 0x0 0x40000>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 89 4>;
+ xlnx,sdfec-code = "ldpc";
+ xlnx,sdfec-din-words = <0>;
+ xlnx,sdfec-din-width = <2>;
+ xlnx,sdfec-dout-words = <0>;
+ xlnx,sdfec-dout-width = <1>;
+ };
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:05:52

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 08/12] misc: xilinx_sdfec: Add ability to get/set config

- Add capability to get SD-FEC config data using ioctl
XSDFEC_GET_CONFIG.

- Add capability to set SD-FEC data order using ioctl
SDFEC_SET_ORDER. The order of data blocks can change
from input to output or to be maintained.

- Add capability to set SD-FEC bypass option using ioctl
XSDFEC_SET_BYPASS. The bypass mode is when HW is
configured to skip SD-FEC engine and redirect input
directly to output.

- Add capability to set SD-FEC active state using ioctl
XSDFEC_IS_ACTIVE. Support for ioctl which puts SD-FEC
in active or non-active state.

Reviewed-by: Michal Simek <[email protected]>
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 | 86 ++++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 57 ++++++++++++++++++++++++++
2 files changed, 143 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index 088ad9d..e980b70 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -309,6 +309,19 @@ static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
return 0;
}

+static int xsdfec_get_config(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ int err;
+
+ err = copy_to_user(arg, &xsdfec->config, sizeof(xsdfec->config));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ err = -EFAULT;
+ }
+ return err;
+}
+
static int xsdfec_set_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
{
struct xsdfec_turbo turbo;
@@ -670,6 +683,67 @@ static int xsdfec_add_ldpc(struct xsdfec_dev *xsdfec, void __user *arg)
return ret;
}

+static int xsdfec_set_order(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ bool order_invalid;
+ enum xsdfec_order order = *((enum xsdfec_order *)arg);
+
+ order_invalid = (order != XSDFEC_MAINTAIN_ORDER) &&
+ (order != XSDFEC_OUT_OF_ORDER);
+ if (order_invalid) {
+ dev_err(xsdfec->dev, "%s invalid order value %d for SDFEC%d",
+ __func__, order, xsdfec->config.fec_id);
+ return -EINVAL;
+ }
+
+ /* Verify Device has not started */
+ if (xsdfec->state == XSDFEC_STARTED) {
+ dev_err(xsdfec->dev,
+ "%s attempting to set Order while started for SDFEC%d",
+ __func__, xsdfec->config.fec_id);
+ return -EIO;
+ }
+
+ xsdfec_regwrite(xsdfec, XSDFEC_ORDER_ADDR, order);
+
+ xsdfec->config.order = order;
+
+ return 0;
+}
+
+static int xsdfec_set_bypass(struct xsdfec_dev *xsdfec, bool __user *arg)
+{
+ bool bypass = *arg;
+
+ /* Verify Device has not started */
+ if (xsdfec->state == XSDFEC_STARTED) {
+ dev_err(xsdfec->dev,
+ "%s attempting to set bypass while started for SDFEC%d",
+ __func__, xsdfec->config.fec_id);
+ return -EIO;
+ }
+
+ if (bypass)
+ xsdfec_regwrite(xsdfec, XSDFEC_BYPASS_ADDR, 1);
+ else
+ xsdfec_regwrite(xsdfec, XSDFEC_BYPASS_ADDR, 0);
+
+ xsdfec->config.bypass = bypass;
+
+ return 0;
+}
+
+static int xsdfec_is_active(struct xsdfec_dev *xsdfec, bool __user *is_active)
+{
+ u32 reg_value;
+
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_ACTIVE_ADDR);
+ /* using a double ! operator instead of casting */
+ *is_active = !!(reg_value & XSDFEC_IS_ACTIVITY_SET);
+
+ return 0;
+}
+
static u32
xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
{
@@ -771,6 +845,9 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
}

switch (cmd) {
+ case XSDFEC_GET_CONFIG:
+ rval = xsdfec_get_config(xsdfec, arg);
+ break;
case XSDFEC_SET_TURBO:
rval = xsdfec_set_turbo(xsdfec, arg);
break;
@@ -780,6 +857,15 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
case XSDFEC_ADD_LDPC_CODE_PARAMS:
rval = xsdfec_add_ldpc(xsdfec, arg);
break;
+ case XSDFEC_SET_ORDER:
+ rval = xsdfec_set_order(xsdfec, arg);
+ break;
+ case XSDFEC_SET_BYPASS:
+ rval = xsdfec_set_bypass(xsdfec, arg);
+ break;
+ case XSDFEC_IS_ACTIVE:
+ rval = xsdfec_is_active(xsdfec, (bool __user *)arg);
+ break;
default:
/* Should not get here */
dev_err(xsdfec->dev, "Undefined SDFEC IOCTL");
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index b70dbff..c6584a4 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -310,6 +310,19 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
#define XSDFEC_ADD_LDPC_CODE_PARAMS \
_IOW(XSDFEC_MAGIC, 5, struct xsdfec_ldpc_params *)
/**
+ * DOC: XSDFEC_GET_CONFIG
+ * @Parameters
+ *
+ * @struct xsdfec_config *
+ * Pointer to the &struct xsdfec_config that contains the current
+ * configuration settings of the SD-FEC Block
+ *
+ * @Description
+ *
+ * ioctl that returns SD-FEC core configuration
+ */
+#define XSDFEC_GET_CONFIG _IOR(XSDFEC_MAGIC, 6, struct xsdfec_config *)
+/**
* DOC: XSDFEC_GET_TURBO
* @Parameters
*
@@ -322,4 +335,48 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
* ioctl that returns SD-FEC turbo param values
*/
#define XSDFEC_GET_TURBO _IOR(XSDFEC_MAGIC, 7, struct xsdfec_turbo *)
+/**
+ * DOC: XSDFEC_SET_ORDER
+ * @Parameters
+ *
+ * @struct unsigned long *
+ * Pointer to the unsigned long that contains a value from the
+ * @enum xsdfec_order
+ *
+ * @Description
+ *
+ * ioctl that sets order, if order of blocks can change from input to output
+ *
+ * This can only be used when the driver is in the XSDFEC_STOPPED state
+ */
+#define XSDFEC_SET_ORDER _IOW(XSDFEC_MAGIC, 8, unsigned long *)
+/**
+ * DOC: XSDFEC_SET_BYPASS
+ * @Parameters
+ *
+ * @struct bool *
+ * Pointer to bool that sets the bypass value, where false results in
+ * normal operation and false results in the SD-FEC performing the
+ * configured operations (same number of cycles) but output data matches
+ * the input data
+ *
+ * @Description
+ *
+ * ioctl that sets bypass.
+ *
+ * This can only be used when the driver is in the XSDFEC_STOPPED state
+ */
+#define XSDFEC_SET_BYPASS _IOW(XSDFEC_MAGIC, 9, bool *)
+/**
+ * DOC: XSDFEC_IS_ACTIVE
+ * @Parameters
+ *
+ * @struct bool *
+ * Pointer to bool that returns true if the SD-FEC is processing data
+ *
+ * @Description
+ *
+ * ioctl that determines if SD-FEC is processing data
+ */
+#define XSDFEC_IS_ACTIVE _IOR(XSDFEC_MAGIC, 10, bool *)
#endif /* __XILINX_SDFEC_H__ */
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:05:57

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 05/12] 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.

Reviewed-by: Michal Simek <[email protected]>
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 | 307 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 94 ++++++++++++
2 files changed, 401 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index 3407de4..d9ead0b 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -36,6 +36,85 @@ static struct class *xsdfec_class;
static atomic_t xsdfec_ndevs = ATOMIC_INIT(0);
static dev_t xsdfec_devt;

+/* 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
@@ -62,6 +141,7 @@ 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
* @open_count: Count of char device being opened
* @xsdfec_cdev: Character device handle
@@ -73,6 +153,7 @@ struct xsdfec_clks {
struct xsdfec_dev {
void __iomem *regs;
struct device *dev;
+ enum xsdfec_state state;
struct xsdfec_config config;
atomic_t open_count;
struct cdev xsdfec_cdev;
@@ -81,6 +162,65 @@ struct xsdfec_dev {
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 int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
{
struct xsdfec_dev *xsdfec;
@@ -111,6 +251,69 @@ static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
return 0;
}

+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 long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
unsigned long data)
{
@@ -164,6 +367,104 @@ static const struct file_operations xsdfec_fops = {
.unlocked_ioctl = xsdfec_dev_ioctl,
};

+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) {
+ dev_err(dev, "xlnx,sdfec-code not in DT");
+ 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 {
+ dev_err(xsdfec->dev, "Invalid Code in DT");
+ return -EINVAL;
+ }
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-din-words",
+ &din_word_include);
+ if (rval < 0) {
+ dev_err(dev, "xlnx,sdfec-din-words not in DT");
+ return rval;
+ }
+
+ if (din_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX) {
+ xsdfec->config.din_word_include = din_word_include;
+ } else {
+ dev_err(xsdfec->dev, "Invalid DIN Words in DT");
+ return -EINVAL;
+ }
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-din-width", &din_width);
+ if (rval < 0) {
+ dev_err(dev, "xlnx,sdfec-din-width not in DT");
+ 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:
+ dev_err(xsdfec->dev, "Invalid DIN Width in DT");
+ return -EINVAL;
+ }
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-dout-words",
+ &dout_word_include);
+ if (rval < 0) {
+ dev_err(dev, "xlnx,sdfec-dout-words not in DT");
+ return rval;
+ }
+
+ if (dout_word_include < XSDFEC_AXIS_WORDS_INCLUDE_MAX) {
+ xsdfec->config.dout_word_include = dout_word_include;
+ } else {
+ dev_err(xsdfec->dev, "Invalid DOUT Words in DT");
+ return -EINVAL;
+ }
+
+ rval = of_property_read_u32(node, "xlnx,sdfec-dout-width", &dout_width);
+ if (rval < 0) {
+ dev_err(dev, "xlnx,sdfec-dout-width not in DT");
+ 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:
+ dev_err(xsdfec->dev, "Invalid DOUT Width in DT");
+ 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)
{
@@ -316,6 +617,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
index 5de0add..0497b66 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -12,6 +12,33 @@
#define __XILINX_SDFEC_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.
@@ -30,13 +57,80 @@ enum xsdfec_state {
};

/**
+ * 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 {
+ char enable_isr;
+ char enable_ecc_isr;
+};
+
+/**
* struct xsdfec_config - Configuration of SD-FEC core.
* @fec_id: ID of SD-FEC instance. ID is limited to the number of active
* SD-FEC's in the FPGA and is related to the driver instance
* Minor number.
+ * @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 {
s32 fec_id;
+ enum xsdfec_code code;
+ enum xsdfec_order order;
+ char bypass;
+ char 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;
};

/*
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:06:00

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 09/12] misc: xilinx_sdfec: Support poll file operation

Support monitoring and detecting the SD-FEC error events
through IRQ and poll file operation.

The SD-FEC device can detect one-error or multi-error events.
An error triggers an interrupt which creates and run the ONE_SHOT
IRQ thread.
The ONE_SHOT IRQ thread detects type of error and pass that
information to the poll function.
The file_operation callback poll(), collects the events and
updates the statistics accordingly.
The function poll blocks() on waiting queue which can be
unblocked by ONE_SHOT IRQ handling thread.

Support SD-FEC interrupt set ioctl callback.
The SD-FEC can detect two type of errors: coding errors (ECC) and
a data interface errors (TLAST).
The errors are events which can trigger an IRQ if enabled.
The driver can monitor and detect these errors through IRQ.
Also the driver updates the statistical data.

Reviewed-by: Michal Simek <[email protected]>
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 | 286 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 13 ++
2 files changed, 299 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index e980b70..b48a881 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -201,8 +201,15 @@ struct xsdfec_clks {
* @dev: pointer to device struct
* @state: State of the SDFEC device
* @config: Configuration of the SDFEC device
+ * @state_updated: indicates State updated by interrupt handler
+ * @stats_updated: indicates Stats updated by interrupt handler
+ * @isr_err_count: Count of ISR errors
+ * @cecc_count: Count of Correctable ECC errors (SBE)
+ * @uecc_count: Count of Uncorrectable ECC errors (MBE)
* @open_count: Count of char device being opened
+ * @irq: IRQ number
* @xsdfec_cdev: Character device handle
+ * @waitq: Driver wait queue
* @irq_lock: Driver spinlock
* @clks: Clocks managed by the SDFEC driver
*
@@ -213,8 +220,15 @@ struct xsdfec_dev {
struct device *dev;
enum xsdfec_state state;
struct xsdfec_config config;
+ bool state_updated;
+ bool stats_updated;
+ atomic_t isr_err_count;
+ atomic_t cecc_count;
+ atomic_t uecc_count;
atomic_t open_count;
+ int irq;
struct cdev xsdfec_cdev;
+ wait_queue_head_t waitq;
/* Spinlock to protect state_updated and stats_updated */
spinlock_t irq_lock;
struct xsdfec_clks clks;
@@ -322,6 +336,93 @@ static int xsdfec_get_config(struct xsdfec_dev *xsdfec, void __user *arg)
return err;
}

+static int xsdfec_isr_enable(struct xsdfec_dev *xsdfec, bool enable)
+{
+ u32 mask_read;
+
+ if (enable) {
+ /* Enable */
+ xsdfec_regwrite(xsdfec, XSDFEC_IER_ADDR, XSDFEC_ISR_MASK);
+ mask_read = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
+ if (mask_read & XSDFEC_ISR_MASK) {
+ dev_err(xsdfec->dev,
+ "SDFEC enabling irq with IER failed");
+ return -EIO;
+ }
+ } else {
+ /* Disable */
+ xsdfec_regwrite(xsdfec, XSDFEC_IDR_ADDR, XSDFEC_ISR_MASK);
+ mask_read = xsdfec_regread(xsdfec, XSDFEC_IMR_ADDR);
+ if ((mask_read & XSDFEC_ISR_MASK) != XSDFEC_ISR_MASK) {
+ dev_err(xsdfec->dev,
+ "SDFEC disabling irq with IDR failed");
+ return -EIO;
+ }
+ }
+ return 0;
+}
+
+static int xsdfec_ecc_isr_enable(struct xsdfec_dev *xsdfec, bool enable)
+{
+ u32 mask_read;
+
+ if (enable) {
+ /* Enable */
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_IER_ADDR,
+ XSDFEC_ALL_ECC_ISR_MASK);
+ mask_read = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
+ if (mask_read & XSDFEC_ALL_ECC_ISR_MASK) {
+ dev_err(xsdfec->dev,
+ "SDFEC enabling ECC irq with ECC IER failed");
+ return -EIO;
+ }
+ } else {
+ /* Disable */
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_IDR_ADDR,
+ XSDFEC_ALL_ECC_ISR_MASK);
+ mask_read = xsdfec_regread(xsdfec, XSDFEC_ECC_IMR_ADDR);
+ if (!(((mask_read & XSDFEC_ALL_ECC_ISR_MASK) ==
+ XSDFEC_ECC_ISR_MASK) ||
+ ((mask_read & XSDFEC_ALL_ECC_ISR_MASK) ==
+ XSDFEC_PL_INIT_ECC_ISR_MASK))) {
+ dev_err(xsdfec->dev,
+ "SDFEC disable ECC irq with ECC IDR failed");
+ return -EIO;
+ }
+ }
+ return 0;
+}
+
+static int xsdfec_set_irq(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ struct xsdfec_irq irq;
+ int err;
+ int isr_err;
+ int ecc_err;
+
+ err = copy_from_user(&irq, arg, sizeof(irq));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ return -EFAULT;
+ }
+
+ /* Setup tlast related IRQ */
+ isr_err = xsdfec_isr_enable(xsdfec, irq.enable_isr);
+ if (!isr_err)
+ xsdfec->config.irq.enable_isr = irq.enable_isr;
+
+ /* Setup ECC related IRQ */
+ ecc_err = xsdfec_ecc_isr_enable(xsdfec, irq.enable_ecc_isr);
+ if (!ecc_err)
+ xsdfec->config.irq.enable_ecc_isr = irq.enable_ecc_isr;
+
+ if (isr_err < 0 || ecc_err < 0)
+ err = -EIO;
+
+ return err;
+}
+
static int xsdfec_set_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
{
struct xsdfec_turbo turbo;
@@ -848,6 +949,9 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
case XSDFEC_GET_CONFIG:
rval = xsdfec_get_config(xsdfec, arg);
break;
+ case XSDFEC_SET_IRQ:
+ rval = xsdfec_set_irq(xsdfec, arg);
+ break;
case XSDFEC_SET_TURBO:
rval = xsdfec_set_turbo(xsdfec, arg);
break;
@@ -874,11 +978,34 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
return rval;
}

+static unsigned int xsdfec_poll(struct file *file, poll_table *wait)
+{
+ unsigned int mask = 0;
+ struct xsdfec_dev *xsdfec = file->private_data;
+
+ if (!xsdfec)
+ return POLLNVAL | POLLHUP;
+
+ poll_wait(file, &xsdfec->waitq, wait);
+
+ /* XSDFEC ISR detected an error */
+ spin_lock_irq(&xsdfec->irq_lock);
+ if (xsdfec->state_updated)
+ mask |= POLLIN | POLLPRI;
+
+ if (xsdfec->stats_updated)
+ mask |= POLLIN | POLLRDNORM;
+ spin_unlock_irq(&xsdfec->irq_lock);
+
+ return mask;
+}
+
static const struct file_operations xsdfec_fops = {
.owner = THIS_MODULE,
.open = xsdfec_dev_open,
.release = xsdfec_dev_release,
.unlocked_ioctl = xsdfec_dev_ioctl,
+ .poll = xsdfec_poll,
};

static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
@@ -979,6 +1106,146 @@ static int xsdfec_parse_of(struct xsdfec_dev *xsdfec)
return 0;
}

+static void xsdfec_count_and_clear_ecc_multi_errors(struct xsdfec_dev *xsdfec,
+ u32 uecc)
+{
+ u32 uecc_event;
+
+ /* Update ECC ISR error counts */
+ atomic_add(hweight32(uecc), &xsdfec->uecc_count);
+ xsdfec->stats_updated = true;
+
+ /* Clear ECC errors */
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_ISR_ADDR,
+ XSDFEC_ALL_ECC_ISR_MBE_MASK);
+ /* Clear ECC events */
+ if (uecc & XSDFEC_ECC_ISR_MBE_MASK) {
+ uecc_event = uecc >> XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT;
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_ISR_ADDR, uecc_event);
+ } else if (uecc & XSDFEC_PL_INIT_ECC_ISR_MBE_MASK) {
+ uecc_event = uecc >> XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT;
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_ISR_ADDR, uecc_event);
+ }
+}
+
+static void xsdfec_count_and_clear_ecc_single_errors(struct xsdfec_dev *xsdfec,
+ u32 cecc, u32 sbe_mask)
+{
+ /* Update ECC ISR error counts */
+ atomic_add(hweight32(cecc), &xsdfec->cecc_count);
+ xsdfec->stats_updated = true;
+
+ /* Clear ECC errors */
+ xsdfec_regwrite(xsdfec, XSDFEC_ECC_ISR_ADDR, sbe_mask);
+}
+
+static void xsdfec_count_and_clear_isr_errors(struct xsdfec_dev *xsdfec,
+ u32 isr_err)
+{
+ /* Update ISR error counts */
+ atomic_add(hweight32(isr_err), &xsdfec->isr_err_count);
+ xsdfec->stats_updated = true;
+
+ /* Clear ISR error status */
+ xsdfec_regwrite(xsdfec, XSDFEC_ISR_ADDR, XSDFEC_ISR_MASK);
+}
+
+static void xsdfec_update_state_for_isr_err(struct xsdfec_dev *xsdfec)
+{
+ xsdfec->state = XSDFEC_NEEDS_RESET;
+ xsdfec->state_updated = true;
+}
+
+static void xsdfec_update_state_for_ecc_err(struct xsdfec_dev *xsdfec,
+ u32 ecc_err)
+{
+ if (ecc_err & XSDFEC_ECC_ISR_MBE_MASK)
+ xsdfec->state = XSDFEC_NEEDS_RESET;
+ else if (ecc_err & XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
+ xsdfec->state = XSDFEC_PL_RECONFIGURE;
+
+ xsdfec->state_updated = true;
+}
+
+static int xsdfec_get_sbe_mask(u32 ecc_err)
+{
+ u32 sbe_mask = XSDFEC_ALL_ECC_ISR_SBE_MASK;
+
+ if (ecc_err & XSDFEC_ECC_ISR_MBE_MASK) {
+ sbe_mask = (XSDFEC_ECC_ISR_MBE_MASK - ecc_err) >>
+ XSDFEC_ECC_ISR_MBE_TO_EVENT_SHIFT;
+ } else if (ecc_err & XSDFEC_PL_INIT_ECC_ISR_MBE_MASK)
+ sbe_mask = (XSDFEC_PL_INIT_ECC_ISR_MBE_MASK - ecc_err) >>
+ XSDFEC_PL_INIT_ECC_ISR_MBE_TO_EVENT_SHIFT;
+
+ return sbe_mask;
+}
+
+static irqreturn_t xsdfec_irq_thread(int irq, void *dev_id)
+{
+ struct xsdfec_dev *xsdfec = dev_id;
+ irqreturn_t ret = IRQ_HANDLED;
+ u32 ecc_err;
+ u32 isr_err;
+ u32 err_value;
+ u32 sbe_mask;
+
+ WARN_ON(xsdfec->irq != irq);
+
+ /* Mask Interrupts */
+ xsdfec_isr_enable(xsdfec, false);
+ xsdfec_ecc_isr_enable(xsdfec, false);
+
+ /* Read Interrupt Status Registers */
+ ecc_err = xsdfec_regread(xsdfec, XSDFEC_ECC_ISR_ADDR);
+ isr_err = xsdfec_regread(xsdfec, XSDFEC_ISR_ADDR);
+
+ spin_lock(&xsdfec->irq_lock);
+
+ err_value = ecc_err & XSDFEC_ALL_ECC_ISR_MBE_MASK;
+ if (err_value) {
+ dev_err(xsdfec->dev, "Multi-bit error on xsdfec%d",
+ xsdfec->config.fec_id);
+ /* Count and clear multi-bit errors and associated events */
+ xsdfec_count_and_clear_ecc_multi_errors(xsdfec, err_value);
+ xsdfec_update_state_for_ecc_err(xsdfec, ecc_err);
+ }
+
+ /*
+ * Update SBE mask to remove events associated with MBE if present.
+ * If no MBEs are present will return mask for all SBE bits
+ */
+ sbe_mask = xsdfec_get_sbe_mask(err_value);
+ err_value = ecc_err & sbe_mask;
+ if (err_value) {
+ dev_info(xsdfec->dev, "Correctable error on xsdfec%d",
+ xsdfec->config.fec_id);
+ xsdfec_count_and_clear_ecc_single_errors(xsdfec, err_value,
+ sbe_mask);
+ }
+
+ err_value = isr_err & XSDFEC_ISR_MASK;
+ if (err_value) {
+ dev_err(xsdfec->dev,
+ "Tlast,or DIN_WORDS or DOUT_WORDS not correct");
+ xsdfec_count_and_clear_isr_errors(xsdfec, err_value);
+ xsdfec_update_state_for_isr_err(xsdfec);
+ }
+
+ if (xsdfec->state_updated || xsdfec->stats_updated)
+ wake_up_interruptible(&xsdfec->waitq);
+ else
+ ret = IRQ_NONE;
+
+ /* Unmaks Interrupts */
+ xsdfec_isr_enable(xsdfec, true);
+ xsdfec_ecc_isr_enable(xsdfec, true);
+
+ spin_unlock(&xsdfec->irq_lock);
+
+ return ret;
+}
+
static int xsdfec_clk_init(struct platform_device *pdev,
struct xsdfec_clks *clks)
{
@@ -1109,6 +1376,7 @@ static int xsdfec_probe(struct platform_device *pdev)
struct device *dev_create;
struct resource *res;
int err;
+ bool irq_enabled = true;

xsdfec = devm_kzalloc(&pdev->dev, sizeof(*xsdfec), GFP_KERNEL);
if (!xsdfec)
@@ -1131,6 +1399,12 @@ static int xsdfec_probe(struct platform_device *pdev)
goto err_xsdfec_dev;
}

+ xsdfec->irq = platform_get_irq(pdev, 0);
+ if (xsdfec->irq < 0) {
+ dev_dbg(dev, "platform_get_irq failed");
+ irq_enabled = false;
+ }
+
err = xsdfec_parse_of(xsdfec);
if (err < 0)
goto err_xsdfec_dev;
@@ -1140,6 +1414,18 @@ static int xsdfec_probe(struct platform_device *pdev)
/* Save driver private data */
platform_set_drvdata(pdev, xsdfec);

+ if (irq_enabled) {
+ init_waitqueue_head(&xsdfec->waitq);
+ /* Register IRQ thread */
+ err = devm_request_threaded_irq(dev, xsdfec->irq, NULL,
+ xsdfec_irq_thread, IRQF_ONESHOT,
+ "xilinx-sdfec16", xsdfec);
+ if (err < 0) {
+ dev_err(dev, "unable to request IRQ%d", xsdfec->irq);
+ goto err_xsdfec_dev;
+ }
+ }
+
cdev_init(&xsdfec->xsdfec_cdev, &xsdfec_fops);
xsdfec->xsdfec_cdev.owner = THIS_MODULE;
err = cdev_add(&xsdfec->xsdfec_cdev,
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index c6584a4..8dfada9 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -274,6 +274,19 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
*/
#define XSDFEC_MAGIC 'f'
/**
+ * DOC: XSDFEC_SET_IRQ
+ * @Parameters
+ *
+ * @struct xsdfec_irq *
+ * Pointer to the &struct xsdfec_irq that contains the interrupt settings
+ * for the SD-FEC core
+ *
+ * @Description
+ *
+ * ioctl to enable or disable irq
+ */
+#define XSDFEC_SET_IRQ _IOW(XSDFEC_MAGIC, 3, struct xsdfec_irq *)
+/**
* DOC: XSDFEC_SET_TURBO
* @Parameters
*
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:06:08

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 11/12] Docs: misc: xilinx_sdfec: Add documentation

Add SD-FEC driver documentation.

Signed-off-by: Derek Kiernan <[email protected]>
Signed-off-by: Dragan Cvetic <[email protected]>
---
Documentation/misc-devices/index.rst | 1 +
Documentation/misc-devices/xilinx_sdfec.rst | 286 ++++++++++++++++++++++++++++
2 files changed, 287 insertions(+)
create mode 100644 Documentation/misc-devices/xilinx_sdfec.rst

diff --git a/Documentation/misc-devices/index.rst b/Documentation/misc-devices/index.rst
index dfd1f45..b5b4757 100644
--- a/Documentation/misc-devices/index.rst
+++ b/Documentation/misc-devices/index.rst
@@ -15,3 +15,4 @@ fit into other categories.
:maxdepth: 2

ibmvmc
+ xilinx_sdfec
diff --git a/Documentation/misc-devices/xilinx_sdfec.rst b/Documentation/misc-devices/xilinx_sdfec.rst
new file mode 100644
index 0000000..99ec3dd
--- /dev/null
+++ b/Documentation/misc-devices/xilinx_sdfec.rst
@@ -0,0 +1,286 @@
+.. SPDX-License-Identifier: GPL-2.0+
+====================
+Xilinx SD-FEC Driver
+====================
+
+Overview
+========
+
+This driver supports SD-FEC Integrated Block for Zynq |Ultrascale+ (TM)| RFSoCs.
+
+.. |Ultrascale+ (TM)| unicode:: Ultrascale+ U+2122
+ .. with trademark sign
+
+For a full description of SD-FEC core features, see the `SD-FEC Product Guide (PG256) <https://www.xilinx.com/cgi-bin/docs/ipdoc?c=sd_fec;v=latest;d=pg256-sdfec-integrated-block.pdf>`_
+
+This driver supports the following features:
+
+ - Retrieval of the Integrated Block configuration and status information
+ - Configuration of LDPC codes
+ - Configuration of Turbo decoding
+ - Monitoring errors
+
+Missing features, known issues, and limitations of the SD-FEC driver are as
+follows:
+
+ - Only allows a single open file handler to any instance of the driver at any time
+ - Reset of the SD-FEC Integrated Block is not controlled by this driver
+ - Does not support shared LDPC code table wraparound
+
+The device tree entry is described in:
+`linux-xlnx/Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt <https://github.com/Xilinx/linux-xlnx/blob/master/Documentation/devicetree/bindings/misc/xlnx%2Csd-fec.txt>`_
+
+
+Modes of Operation
+------------------
+
+The driver works with the SD-FEC core in two modes of operation:
+
+ - Run-time configuration
+ - Programmable Logic (PL) initialization
+
+
+Run-time Configuration
+~~~~~~~~~~~~~~~~~~~~~~
+
+For Run-time configuration the role of driver is to allow the software application to do the following:
+
+ - Load the configuration parameters for either Turbo decode or LDPC encode or decode
+ - Activate the SD-FEC core
+ - Monitor the SD-FEC core for errors
+ - Retrieve the status and configuration of the SD-FEC core
+
+Programmable Logic (PL) Initialization
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+For PL initialization, supporting logic loads configuration parameters for either
+the Turbo decode or LDPC encode or decode. The role of the driver is to allow
+the software application to do the following:
+
+ - Activate the SD-FEC core
+ - Monitor the SD-FEC core for errors
+ - Retrieve the status and configuration of the SD-FEC core
+
+
+Driver Structure
+================
+
+The driver provides a platform device where the ``probe`` and ``remove``
+operations are provided.
+
+ - probe: Updates configuration register with device-tree entries plus determines the current activate state of the core, for example, is the core bypassed or has the core been started.
+
+
+The driver defines the following driver file operations to provide user
+application interfaces:
+
+ - open: Implements restriction that only a single file descriptor can be open per SD-FEC instance at any time
+ - release: Allows another file descriptor to be open, that is after current file descriptor is closed
+ - poll: Provides a method to monitor for SD-FEC Error events
+ - unlocked_ioctl: Provides the the following ioctl commands that allows the application configure the SD-FEC core:
+
+ - :c:macro:`XSDFEC_START_DEV`
+ - :c:macro:`XSDFEC_STOP_DEV`
+ - :c:macro:`XSDFEC_GET_STATUS`
+ - :c:macro:`XSDFEC_SET_IRQ`
+ - :c:macro:`XSDFEC_SET_TURBO`
+ - :c:macro:`XSDFEC_ADD_LDPC_CODE_PARAMS`
+ - :c:macro:`XSDFEC_GET_CONFIG`
+ - :c:macro:`XSDFEC_SET_ORDER`
+ - :c:macro:`XSDFEC_SET_BYPASS`
+ - :c:macro:`XSDFEC_IS_ACTIVE`
+ - :c:macro:`XSDFEC_CLEAR_STATS`
+ - :c:macro:`XSDFEC_SET_DEFAULT_CONFIG`
+
+
+Driver Usage
+============
+
+
+Overview
+--------
+
+After opening the driver, the user should find out what operations need to be
+performed to configure and activate the SD-FEC core and determine the
+configuration of the driver.
+The following outlines the flow the user should perform:
+
+ - Determine Configuration
+ - Set the order, if not already configured as desired
+ - Set Turbo decode, LPDC encode or decode parameters, depending on how the
+ SD-FEC core is configured plus if the SD-FEC has not been configured for PL
+ initialization
+ - Enable interrupts, if not already enabled
+ - Bypass the SD-FEC core, if required
+ - Start the SD-FEC core if not already started
+ - Get the SD-FEC core status
+ - Monitor for interrupts
+ - Stop the SD-FEC core
+
+
+Note: When monitoring for interrupts if a critical error is detected where a reset is required, the driver will be required to load the default configuration.
+
+
+Determine Configuration
+-----------------------
+
+Determine the configuration of the SD-FEC core by using the ioctl
+:c:macro:`XSDFEC_GET_CONFIG`.
+
+Set the Order
+-------------
+
+Setting the order determines how the order of Blocks can change from input to output.
+
+Setting the order is done by using the ioctl :c:macro:`XSDFEC_SET_ORDER`
+
+Setting the order can only be done if the following restrictions are met:
+
+ - The ``state`` member of struct :c:type:`xsdfec_status <xsdfec_status>` filled by the ioctl :c:macro:`XSDFEC_GET_STATUS` indicates the SD-FEC core has not STARTED
+
+
+Add LDPC Codes
+--------------
+
+The following steps indicate how to add LDPC codes to the SD-FEC core:
+
+ - Use the auto-generated parameters to fill the :c:type:`struct xsdfec_ldpc_params <xsdfec_ldpc_params>` for the desired LDPC code.
+ - Set the SC, QA, and LA table offsets for the LPDC parameters and the parameters in the structure :c:type:`struct xsdfec_ldpc_params <xsdfec_ldpc_params>`
+ - Set the desired Code Id value in the structure :c:type:`struct xsdfec_ldpc_params <xsdfec_ldpc_params>`
+ - Add the LPDC Code Parameters using the ioctl :c:macro:`XSDFEC_ADD_LDPC_CODE_PARAMS`
+ - For the applied LPDC Code Parameter use the function :c:func:`xsdfec_calculate_shared_ldpc_table_entry_size` to calculate the size of shared LPDC code tables. This allows the user to determine the shared table usage so when selecting the table offsets for the next LDPC code parameters unused table areas can be selected.
+ - Repeat for each LDPC code parameter.
+
+Adding LDPC codes can only be done if the following restrictions are met:
+
+ - The ``code`` member of :c:type:`struct xsdfec_config <xsdfec_config>` filled by the ioctl :c:macro:`XSDFEC_GET_CONFIG` indicates the SD-FEC core is configured as LDPC
+ - The ``code_wr_protect`` of :c:type:`struct xsdfec_config <xsdfec_config>` filled by the ioctl :c:macro:`XSDFEC_GET_CONFIG` indicates that write protection is not enabled
+ - The ``state`` member of struct :c:type:`xsdfec_status <xsdfec_status>` filled by the ioctl :c:macro:`XSDFEC_GET_STATUS` indicates the SD-FEC core has not started
+
+Set Turbo Decode
+----------------
+
+Configuring the Turbo decode parameters is done by using the ioctl :c:macro:`XSDFEC_SET_TURBO` using auto-generated parameters to fill the :c:type:`struct xsdfec_turbo <xsdfec_turbo>` for the desired Turbo code.
+
+Adding Turbo decode can only be done if the following restrictions are met:
+
+ - The ``code`` member of :c:type:`struct xsdfec_config <xsdfec_config>` filled by the ioctl :c:macro:`XSDFEC_GET_CONFIG` indicates the SD-FEC core is configured as TURBO
+ - The ``state`` member of struct :c:type:`xsdfec_status <xsdfec_status>` filled by the ioctl :c:macro:`XSDFEC_GET_STATUS` indicates the SD-FEC core has not STARTED
+
+Enable Interrupts
+-----------------
+
+Enabling or disabling interrupts is done by using the ioctl :c:macro:`XSDFEC_SET_IRQ`. The members of the parameter passed, :c:type:`struct xsdfec_irq <xsdfec_irq>`, to the ioctl are used to set and clear different categories of interrupts. The category of interrupt is controlled as following:
+
+ - ``enable_isr`` controls the ``tlast`` interrupts
+ - ``enable_ecc_isr`` controls the ECC interrupts
+
+If the ``code`` member of :c:type:`struct xsdfec_config <xsdfec_config>` filled by the ioctl :c:macro:`XSDFEC_GET_CONFIG` indicates the SD-FEC core is configured as TURBO then the enabling ECC errors is not required.
+
+Bypass the SD-FEC
+-----------------
+
+Bypassing the SD-FEC is done by using the ioctl :c:macro:`XSDFEC_SET_BYPASS`
+
+Bypassing the SD-FEC can only be done if the following restrictions are met:
+
+ - The ``state`` member of :c:type:`struct xsdfec_status <xsdfec_status>` filled by the ioctl :c:macro:`XSDFEC_GET_STATUS` indicates the SD-FEC core has not STARTED
+
+Start the SD-FEC core
+---------------------
+
+Start the SD-FEC core by using the ioctl :c:macro:`XSDFEC_START_DEV`
+
+Get SD-FEC Status
+-----------------
+
+Get the SD-FEC status of the device by using the ioctl :c:macro:`XSDFEC_GET_STATUS`, which will fill the :c:type:`struct xsdfec_status <xsdfec_status>`
+
+Monitor for Interrupts
+----------------------
+
+ - Use the poll system call to monitor for an interrupt. The poll system call waits for an interrupt to wake it up or times out if no interrupt occurs.
+ - On return Poll ``revents`` will indicate whether stats and/or state have been updated
+ - ``POLLPRI`` indicates a critical error and the user should use :c:macro:`XSDFEC_GET_STATUS` and :c:macro:`XSDFEC_GET_STATS` to confirm
+ - ``POLLRDNORM`` indicates a non-critical error has occurred and the user should use :c:macro:`XSDFEC_GET_STATS` to confirm
+ - Get stats by using the ioctl :c:macro:`XSDFEC_GET_STATS`
+ - For critical error the ``isr_err_count`` or ``uecc_count`` member of :c:type:`struct xsdfec_stats <xsdfec_stats>` is non-zero
+ - For non-critical errors the ``cecc_count`` member of :c:type:`struct xsdfec_stats <xsdfec_stats>` is non-zero
+ - Get state by using the ioctl :c:macro:`XSDFEC_GET_STATUS`
+ - For a critical error the ``state`` of :c:type:`xsdfec_status <xsdfec_status>` will indicate a Reset Is Required
+ - Clear stats by using the ioctl :c:macro:`XSDFEC_CLEAR_STATS`
+
+If a critical error is detected where a reset is required. The application is required to call the ioctl :c:macro:`XSDFEC_SET_DEFAULT_CONFIG`, after the reset and it is not required to call the ioctl :c:macro:`XSDFEC_STOP_DEV`
+
+Note: Using poll system call prevents busy looping using :c:macro:`XSDFEC_GET_STATS` and :c:macro:`XSDFEC_GET_STATUS`
+
+Stop the SD-FEC Core
+---------------------
+
+Stop the device by using the ioctl :c:macro:`XSDFEC_STOP_DEV`
+
+Set the Default Configuration
+-----------------------------
+
+Load default configuration by using the ioctl :c:macro:`XSDFEC_SET_DEFAULT_CONFIG` to restore the driver.
+
+Driver IOCTLs
+==============
+
+.. c:macro:: XSDFEC_START_DEV
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_START_DEV
+
+.. c:macro:: XSDFEC_STOP_DEV
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_STOP_DEV
+
+.. c:macro:: XSDFEC_GET_STATUS
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_GET_STATUS
+
+.. c:macro:: XSDFEC_SET_IRQ
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_SET_IRQ
+
+.. c:macro:: XSDFEC_SET_TURBO
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_SET_TURBO
+
+.. c:macro:: XSDFEC_ADD_LDPC_CODE_PARAMS
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_ADD_LDPC_CODE_PARAMS
+
+.. c:macro:: XSDFEC_GET_CONFIG
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_GET_CONFIG
+
+.. c:macro:: XSDFEC_SET_ORDER
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_SET_ORDER
+
+.. c:macro:: XSDFEC_SET_BYPASS
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_SET_BYPASS
+
+.. c:macro:: XSDFEC_IS_ACTIVE
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_IS_ACTIVE
+
+.. c:macro:: XSDFEC_CLEAR_STATS
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_CLEAR_STATS
+
+.. c:macro:: XSDFEC_GET_STATS
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_GET_STATS
+
+.. c:macro:: XSDFEC_SET_DEFAULT_CONFIG
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :doc: XSDFEC_SET_DEFAULT_CONFIG
+
+Driver Type Definitions
+=======================
+
+.. kernel-doc:: include/uapi/misc/xilinx_sdfec.h
+ :internal:
\ No newline at end of file
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:06:14

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 07/12] misc: xilinx_sdfec: Add ability to configure LDPC

Add the capability to configure LDPC mode via the ioctl
XSDFEC_ADD_LDPC_CODE_PARAMS. The ioctl set the parameters
and tables for the LDPC FEC algorithm.

Reviewed-by: Michal Simek <[email protected]>
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 | 345 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 114 +++++++++++++
2 files changed, 459 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index 58b1c57..088ad9d 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -121,6 +121,58 @@ static dev_t xsdfec_devt;
#define XSDFEC_TURBO_SCALE_BIT_POS (8)
#define XSDFEC_TURBO_SCALE_MAX (15)

+/* REG0 Register */
+#define XSDFEC_LDPC_CODE_REG0_ADDR_BASE (0x2000)
+#define XSDFEC_LDPC_CODE_REG0_ADDR_HIGH (0x27F0)
+#define XSDFEC_REG0_N_MIN (4)
+#define XSDFEC_REG0_N_MAX (32768)
+#define XSDFEC_REG0_N_MUL_P (256)
+#define XSDFEC_REG0_N_LSB (0)
+#define XSDFEC_REG0_K_MIN (2)
+#define XSDFEC_REG0_K_MAX (32766)
+#define XSDFEC_REG0_K_MUL_P (256)
+#define XSDFEC_REG0_K_LSB (16)
+
+/* REG1 Register */
+#define XSDFEC_LDPC_CODE_REG1_ADDR_BASE (0x2004)
+#define XSDFEC_LDPC_CODE_REG1_ADDR_HIGH (0x27f4)
+#define XSDFEC_REG1_PSIZE_MIN (2)
+#define XSDFEC_REG1_PSIZE_MAX (512)
+#define XSDFEC_REG1_NO_PACKING_MASK (0x400)
+#define XSDFEC_REG1_NO_PACKING_LSB (10)
+#define XSDFEC_REG1_NM_MASK (0xFF800)
+#define XSDFEC_REG1_NM_LSB (11)
+#define XSDFEC_REG1_BYPASS_MASK (0x100000)
+
+/* REG2 Register */
+#define XSDFEC_LDPC_CODE_REG2_ADDR_BASE (0x2008)
+#define XSDFEC_LDPC_CODE_REG2_ADDR_HIGH (0x27f8)
+#define XSDFEC_REG2_NLAYERS_MIN (1)
+#define XSDFEC_REG2_NLAYERS_MAX (256)
+#define XSDFEC_REG2_NNMQC_MASK (0xFFE00)
+#define XSDFEC_REG2_NMQC_LSB (9)
+#define XSDFEC_REG2_NORM_TYPE_MASK (0x100000)
+#define XSDFEC_REG2_NORM_TYPE_LSB (20)
+#define XSDFEC_REG2_SPECIAL_QC_MASK (0x200000)
+#define XSDFEC_REG2_SPEICAL_QC_LSB (21)
+#define XSDFEC_REG2_NO_FINAL_PARITY_MASK (0x400000)
+#define XSDFEC_REG2_NO_FINAL_PARITY_LSB (22)
+#define XSDFEC_REG2_MAX_SCHEDULE_MASK (0x1800000)
+#define XSDFEC_REG2_MAX_SCHEDULE_LSB (23)
+
+/* REG3 Register */
+#define XSDFEC_LDPC_CODE_REG3_ADDR_BASE (0x200C)
+#define XSDFEC_LDPC_CODE_REG3_ADDR_HIGH (0x27FC)
+#define XSDFEC_REG3_LA_OFF_LSB (8)
+#define XSDFEC_REG3_QC_OFF_LSB (16)
+
+#define XSDFEC_LDPC_REG_JUMP (0x10)
+#define XSDFEC_REG_WIDTH_JUMP (4)
+
+#define XSDFEC_SC_TABLE_DEPTH (0x3FC)
+#define XSDFEC_LA_TABLE_DEPTH (0xFFC)
+#define XSDFEC_QC_TABLE_DEPTH (0x7FFC)
+
/**
* struct xsdfec_clks - For managing SD-FEC clocks
* @core_clk: Main processing clock for core
@@ -328,6 +380,296 @@ static int xsdfec_get_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
return err;
}

+static int xsdfec_reg0_write(struct xsdfec_dev *xsdfec, u32 n, u32 k, u32 psize,
+ u32 offset)
+{
+ u32 wdata;
+
+ if (n < XSDFEC_REG0_N_MIN || n > XSDFEC_REG0_N_MAX ||
+ (n > XSDFEC_REG0_N_MUL_P * psize) || n <= k || ((n % psize) != 0)) {
+ dev_err(xsdfec->dev, "N value is not in range");
+ return -EINVAL;
+ }
+ n <<= XSDFEC_REG0_N_LSB;
+
+ if (k < XSDFEC_REG0_K_MIN || k > XSDFEC_REG0_K_MAX ||
+ (k > XSDFEC_REG0_K_MUL_P * psize) || ((k % psize) != 0)) {
+ dev_err(xsdfec->dev, "K value is not in range");
+ return -EINVAL;
+ }
+ k = k << XSDFEC_REG0_K_LSB;
+ wdata = k | n;
+
+ if (XSDFEC_LDPC_CODE_REG0_ADDR_BASE + (offset * XSDFEC_LDPC_REG_JUMP) >
+ XSDFEC_LDPC_CODE_REG0_ADDR_HIGH) {
+ dev_err(xsdfec->dev, "Writing outside of LDPC reg0 space 0x%x",
+ XSDFEC_LDPC_CODE_REG0_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP));
+ return -EINVAL;
+ }
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_CODE_REG0_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP),
+ wdata);
+ return 0;
+}
+
+static int xsdfec_reg1_write(struct xsdfec_dev *xsdfec, u32 psize,
+ u32 no_packing, u32 nm, u32 offset)
+{
+ u32 wdata;
+
+ if (psize < XSDFEC_REG1_PSIZE_MIN || psize > XSDFEC_REG1_PSIZE_MAX) {
+ dev_err(xsdfec->dev, "Psize is not in range");
+ return -EINVAL;
+ }
+
+ if (no_packing != 0 && no_packing != 1)
+ dev_err(xsdfec->dev, "No-packing bit register invalid");
+ no_packing = ((no_packing << XSDFEC_REG1_NO_PACKING_LSB) &
+ XSDFEC_REG1_NO_PACKING_MASK);
+
+ if (nm & ~(XSDFEC_REG1_NM_MASK >> XSDFEC_REG1_NM_LSB))
+ dev_err(xsdfec->dev, "NM is beyond 10 bits");
+ nm = (nm << XSDFEC_REG1_NM_LSB) & XSDFEC_REG1_NM_MASK;
+
+ wdata = nm | no_packing | psize;
+ if (XSDFEC_LDPC_CODE_REG1_ADDR_BASE + (offset * XSDFEC_LDPC_REG_JUMP) >
+ XSDFEC_LDPC_CODE_REG1_ADDR_HIGH) {
+ dev_err(xsdfec->dev, "Writing outside of LDPC reg1 space 0x%x",
+ XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP));
+ return -EINVAL;
+ }
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP),
+ wdata);
+ return 0;
+}
+
+static int xsdfec_reg2_write(struct xsdfec_dev *xsdfec, u32 nlayers, u32 nmqc,
+ u32 norm_type, u32 special_qc, u32 no_final_parity,
+ u32 max_schedule, u32 offset)
+{
+ u32 wdata;
+
+ if (nlayers < XSDFEC_REG2_NLAYERS_MIN ||
+ nlayers > XSDFEC_REG2_NLAYERS_MAX) {
+ dev_err(xsdfec->dev, "Nlayers is not in range");
+ return -EINVAL;
+ }
+
+ if (nmqc & ~(XSDFEC_REG2_NNMQC_MASK >> XSDFEC_REG2_NMQC_LSB))
+ dev_err(xsdfec->dev, "NMQC exceeds 11 bits");
+ nmqc = (nmqc << XSDFEC_REG2_NMQC_LSB) & XSDFEC_REG2_NNMQC_MASK;
+
+ if (norm_type > 1)
+ dev_err(xsdfec->dev, "Norm type is invalid");
+ norm_type = ((norm_type << XSDFEC_REG2_NORM_TYPE_LSB) &
+ XSDFEC_REG2_NORM_TYPE_MASK);
+ if (special_qc > 1)
+ dev_err(xsdfec->dev, "Special QC in invalid");
+ special_qc = ((special_qc << XSDFEC_REG2_SPEICAL_QC_LSB) &
+ XSDFEC_REG2_SPECIAL_QC_MASK);
+
+ if (no_final_parity > 1)
+ dev_err(xsdfec->dev, "No final parity check invalid");
+ no_final_parity =
+ ((no_final_parity << XSDFEC_REG2_NO_FINAL_PARITY_LSB) &
+ XSDFEC_REG2_NO_FINAL_PARITY_MASK);
+ if (max_schedule &
+ ~(XSDFEC_REG2_MAX_SCHEDULE_MASK >> XSDFEC_REG2_MAX_SCHEDULE_LSB))
+ dev_err(xsdfec->dev, "Max Schdule exceeds 2 bits");
+ max_schedule = ((max_schedule << XSDFEC_REG2_MAX_SCHEDULE_LSB) &
+ XSDFEC_REG2_MAX_SCHEDULE_MASK);
+
+ wdata = (max_schedule | no_final_parity | special_qc | norm_type |
+ nmqc | nlayers);
+
+ if (XSDFEC_LDPC_CODE_REG2_ADDR_BASE + (offset * XSDFEC_LDPC_REG_JUMP) >
+ XSDFEC_LDPC_CODE_REG2_ADDR_HIGH) {
+ dev_err(xsdfec->dev, "Writing outside of LDPC reg2 space 0x%x",
+ XSDFEC_LDPC_CODE_REG2_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP));
+ return -EINVAL;
+ }
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_CODE_REG2_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP),
+ wdata);
+ return 0;
+}
+
+static int xsdfec_reg3_write(struct xsdfec_dev *xsdfec, u8 sc_off, u8 la_off,
+ u16 qc_off, u32 offset)
+{
+ u32 wdata;
+
+ wdata = ((qc_off << XSDFEC_REG3_QC_OFF_LSB) |
+ (la_off << XSDFEC_REG3_LA_OFF_LSB) | sc_off);
+ if (XSDFEC_LDPC_CODE_REG3_ADDR_BASE + (offset * XSDFEC_LDPC_REG_JUMP) >
+ XSDFEC_LDPC_CODE_REG3_ADDR_HIGH) {
+ dev_err(xsdfec->dev, "Writing outside of LDPC reg3 space 0x%x",
+ XSDFEC_LDPC_CODE_REG3_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP));
+ return -EINVAL;
+ }
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_CODE_REG3_ADDR_BASE +
+ (offset * XSDFEC_LDPC_REG_JUMP),
+ wdata);
+ return 0;
+}
+
+static int xsdfec_sc_table_write(struct xsdfec_dev *xsdfec, u32 offset,
+ u32 *sc_ptr, u32 len)
+{
+ u32 reg;
+
+ /*
+ * Writes that go beyond the length of
+ * Shared Scale(SC) table should fail
+ */
+ if ((XSDFEC_REG_WIDTH_JUMP * (offset + len)) > XSDFEC_SC_TABLE_DEPTH) {
+ dev_err(xsdfec->dev, "Write exceeds SC table length");
+ return -EINVAL;
+ }
+
+ for (reg = 0; reg < len; reg++) {
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_SC_TABLE_ADDR_BASE +
+ (offset + reg) * XSDFEC_REG_WIDTH_JUMP,
+ sc_ptr[reg]);
+ }
+ return reg;
+}
+
+static int xsdfec_la_table_write(struct xsdfec_dev *xsdfec, u32 offset,
+ u32 *la_ptr, u32 len)
+{
+ u32 reg;
+
+ if (XSDFEC_REG_WIDTH_JUMP * (offset + len) > XSDFEC_LA_TABLE_DEPTH) {
+ dev_err(xsdfec->dev, "Write exceeds LA table length");
+ return -EINVAL;
+ }
+
+ for (reg = 0; reg < len; reg++) {
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_LA_TABLE_ADDR_BASE +
+ (offset + reg) * XSDFEC_REG_WIDTH_JUMP,
+ la_ptr[reg]);
+ }
+ return reg;
+}
+
+static int xsdfec_qc_table_write(struct xsdfec_dev *xsdfec, u32 offset,
+ u32 *qc_ptr, u32 len)
+{
+ u32 reg;
+
+ if (XSDFEC_REG_WIDTH_JUMP * (offset + len) > XSDFEC_QC_TABLE_DEPTH) {
+ dev_err(xsdfec->dev, "Write exceeds QC table length");
+ return -EINVAL;
+ }
+
+ for (reg = 0; reg < len; reg++) {
+ xsdfec_regwrite(xsdfec,
+ XSDFEC_LDPC_QC_TABLE_ADDR_BASE +
+ (offset + reg) * XSDFEC_REG_WIDTH_JUMP,
+ qc_ptr[reg]);
+ }
+
+ return reg;
+}
+
+static int xsdfec_add_ldpc(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ struct xsdfec_ldpc_params *ldpc;
+ int ret;
+
+ ldpc = kzalloc(sizeof(*ldpc), GFP_KERNEL);
+ if (!ldpc)
+ return -ENOMEM;
+
+ ret = copy_from_user(ldpc, arg, sizeof(*ldpc));
+ if (ret) {
+ dev_err(xsdfec->dev, "%s failed to copy from user for SDFEC%d",
+ __func__, xsdfec->config.fec_id);
+ goto err_out;
+ }
+ if (xsdfec->config.code == XSDFEC_TURBO_CODE) {
+ dev_err(xsdfec->dev,
+ "%s: Unable to write LDPC to SDFEC%d check DT",
+ __func__, xsdfec->config.fec_id);
+ ret = -EIO;
+ goto err_out;
+ }
+
+ /* Verify Device has not started */
+ if (xsdfec->state == XSDFEC_STARTED) {
+ dev_err(xsdfec->dev,
+ "%s attempting to write LDPC code while started for SDFEC%d",
+ __func__, xsdfec->config.fec_id);
+ ret = -EIO;
+ goto err_out;
+ }
+
+ if (xsdfec->config.code_wr_protect) {
+ dev_err(xsdfec->dev,
+ "%s writing LDPC code while Code Write Protection enabled for SDFEC%d",
+ __func__, xsdfec->config.fec_id);
+ ret = -EIO;
+ goto err_out;
+ }
+
+ /* Write Reg 0 */
+ ret = xsdfec_reg0_write(xsdfec, ldpc->n, ldpc->k, ldpc->psize,
+ ldpc->code_id);
+ if (ret)
+ goto err_out;
+
+ /* Write Reg 1 */
+ ret = xsdfec_reg1_write(xsdfec, ldpc->psize, ldpc->no_packing, ldpc->nm,
+ ldpc->code_id);
+ if (ret)
+ goto err_out;
+
+ /* Write Reg 2 */
+ ret = xsdfec_reg2_write(xsdfec, ldpc->nlayers, ldpc->nmqc,
+ ldpc->norm_type, ldpc->special_qc,
+ ldpc->no_final_parity, ldpc->max_schedule,
+ ldpc->code_id);
+ if (ret)
+ goto err_out;
+
+ /* Write Reg 3 */
+ ret = xsdfec_reg3_write(xsdfec, ldpc->sc_off, ldpc->la_off,
+ ldpc->qc_off, ldpc->code_id);
+ if (ret)
+ goto err_out;
+
+ /* Write Shared Codes */
+ ret = xsdfec_sc_table_write(xsdfec, ldpc->sc_off, ldpc->sc_table,
+ ldpc->nlayers);
+ if (ret < 0)
+ goto err_out;
+
+ ret = xsdfec_la_table_write(xsdfec, 4 * ldpc->la_off, ldpc->la_table,
+ ldpc->nlayers);
+ if (ret < 0)
+ goto err_out;
+
+ ret = xsdfec_qc_table_write(xsdfec, 4 * ldpc->qc_off, ldpc->qc_table,
+ ldpc->nqc);
+ if (ret > 0)
+ ret = 0;
+err_out:
+ kfree(ldpc);
+ return ret;
+}
+
static u32
xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
{
@@ -435,6 +777,9 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
case XSDFEC_GET_TURBO:
rval = xsdfec_get_turbo(xsdfec, arg);
break;
+ case XSDFEC_ADD_LDPC_CODE_PARAMS:
+ rval = xsdfec_add_ldpc(xsdfec, arg);
+ break;
default:
/* Should not get here */
dev_err(xsdfec->dev, "Undefined SDFEC IOCTL");
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index 1a15771..b70dbff 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -11,6 +11,14 @@
#ifndef __XILINX_SDFEC_H__
#define __XILINX_SDFEC_H__

+/* Shared LDPC Tables */
+#define XSDFEC_LDPC_SC_TABLE_ADDR_BASE (0x10000)
+#define XSDFEC_LDPC_SC_TABLE_ADDR_HIGH (0x103FC)
+#define XSDFEC_LDPC_LA_TABLE_ADDR_BASE (0x18000)
+#define XSDFEC_LDPC_LA_TABLE_ADDR_HIGH (0x18FFC)
+#define XSDFEC_LDPC_QC_TABLE_ADDR_BASE (0x20000)
+#define XSDFEC_LDPC_QC_TABLE_ADDR_HIGH (0x27FFC)
+
/**
* enum xsdfec_code - Code Type.
* @XSDFEC_TURBO_CODE: Driver is configured for Turbo mode.
@@ -125,6 +133,56 @@ struct xsdfec_turbo {
};

/**
+ * struct xsdfec_ldpc_params - User data for LDPC codes.
+ * @n: Number of code word bits
+ * @k: Number of information bits
+ * @psize: Size of sub-matrix
+ * @nlayers: Number of layers in code
+ * @nqc: Quasi Cyclic Number
+ * @nmqc: Number of M-sized QC operations in parity check matrix
+ * @nm: Number of M-size vectors in N
+ * @norm_type: Normalization required or not
+ * @no_packing: Determines if multiple QC ops should be performed
+ * @special_qc: Sub-Matrix property for Circulant weight > 0
+ * @no_final_parity: Decide if final parity check needs to be performed
+ * @max_schedule: Experimental code word scheduling limit
+ * @sc_off: SC offset
+ * @la_off: LA offset
+ * @qc_off: QC offset
+ * @sc_table: SC Table
+ * @la_table: LA Table
+ * @qc_table: QC Table
+ * @code_id: LDPC Code
+ *
+ * This structure describes the LDPC code that is passed to the driver by the
+ * application.
+ */
+struct xsdfec_ldpc_params {
+ u32 n;
+ u32 k;
+ u32 psize;
+ u32 nlayers;
+ u32 nqc;
+ u32 nmqc;
+ u32 nm;
+ u32 norm_type;
+ u32 no_packing;
+ u32 special_qc;
+ u32 no_final_parity;
+ u32 max_schedule;
+ u32 sc_off;
+ u32 la_off;
+ u32 qc_off;
+ u32 sc_table[XSDFEC_LDPC_SC_TABLE_ADDR_HIGH -
+ XSDFEC_LDPC_SC_TABLE_ADDR_BASE];
+ u32 la_table[XSDFEC_LDPC_LA_TABLE_ADDR_HIGH -
+ XSDFEC_LDPC_LA_TABLE_ADDR_BASE];
+ u32 qc_table[XSDFEC_LDPC_QC_TABLE_ADDR_HIGH -
+ XSDFEC_LDPC_QC_TABLE_ADDR_BASE];
+ u16 code_id;
+};
+
+/**
* struct xsdfec_status - Status of SD-FEC core.
* @fec_id: ID of SD-FEC instance. ID is limited to the number of active
* SD-FEC's in the FPGA and is related to the driver instance
@@ -176,6 +234,41 @@ struct xsdfec_config {
struct xsdfec_irq irq;
};

+/**
+ * struct xsdfec_ldpc_param_table_sizes - Used to store sizes of SD-FEC table
+ * entries for an individual LPDC code
+ * parameter.
+ * @sc_size: Size of SC table used
+ * @la_size: Size of LA table used
+ * @qc_size: Size of QC table used
+ */
+struct xsdfec_ldpc_param_table_sizes {
+ u32 sc_size;
+ u32 la_size;
+ u32 qc_size;
+};
+
+/**
+ * xsdfec_calculate_shared_ldpc_table_entry_size - Calculates shared code
+ * table sizes.
+ * @ldpc: Pointer to the LPDC Code Parameters
+ * @table_sizes: Pointer to structure containing the calculated table sizes
+ *
+ * Calculates the size of shared LDPC code tables used for a specified LPDC code
+ * parameters.
+ */
+inline void
+xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
+ struct xsdfec_ldpc_param_table_sizes *table_sizes)
+{
+ /* Calculate the sc_size in 32 bit words */
+ table_sizes->sc_size = (ldpc->nlayers + 3) >> 2;
+ /* Calculate the la_size in 256 bit words */
+ table_sizes->la_size = ((ldpc->nlayers << 2) + 15) >> 4;
+ /* Calculate the qc_size in 256 bit words */
+ table_sizes->qc_size = ((ldpc->nqc << 2) + 15) >> 4;
+}
+
/*
* XSDFEC IOCTL List
*/
@@ -196,6 +289,27 @@ struct xsdfec_config {
*/
#define XSDFEC_SET_TURBO _IOW(XSDFEC_MAGIC, 4, struct xsdfec_turbo *)
/**
+ * DOC: XSDFEC_ADD_LDPC_CODE_PARAMS
+ * @Parameters
+ *
+ * @struct xsdfec_ldpc_params *
+ * Pointer to the &struct xsdfec_ldpc_params that contains the LDPC code
+ * parameters to be added to the SD-FEC Block
+ *
+ * @Description
+ * ioctl to add an LDPC code to the SD-FEC LDPC codes
+ *
+ * This can only be used when:
+ *
+ * - Driver is in the XSDFEC_STOPPED state
+ *
+ * - SD-FEC core is configured as LPDC
+ *
+ * - SD-FEC Code Write Protection is disabled
+ */
+#define XSDFEC_ADD_LDPC_CODE_PARAMS \
+ _IOW(XSDFEC_MAGIC, 5, struct xsdfec_ldpc_params *)
+/**
* DOC: XSDFEC_GET_TURBO
* @Parameters
*
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:06:31

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 10/12] misc: xilinx_sdfec: Add stats & status ioctls

SD-FEC statistic data are:
- count of data interface errors (isr_err_count)
- count of Correctable ECC errors (cecc_count)
- count of Uncorrectable ECC errors (uecc_count)

Add support:
1. clear stats ioctl callback which clears collected
statistic data,
2. get stats ioctl callback which reads a collected
statistic data,
3. set default configuration ioctl callback,
4. start ioctl callback enables SD-FEC HW,
5. stop ioctl callback disables SD-FEC HW.

In a failed state driver enables the following ioctls:
- get status
- get statistics
- clear stats
- set default SD-FEC device configuration

Reviewed-by: Michal Simek <[email protected]>
Tested-by: Santhosh Dyavanapally <[email protected]>
Tested by: Punnaiah Choudary Kalluri <[email protected]>
Tested-by: Derek Kiernan <[email protected]>
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 | 128 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 75 +++++++++++++++++++++++
2 files changed, 203 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index b48a881..4e29e29 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -201,6 +201,7 @@ struct xsdfec_clks {
* @dev: pointer to device struct
* @state: State of the SDFEC device
* @config: Configuration of the SDFEC device
+ * @intr_enabled: indicates IRQ enabled
* @state_updated: indicates State updated by interrupt handler
* @stats_updated: indicates Stats updated by interrupt handler
* @isr_err_count: Count of ISR errors
@@ -220,6 +221,7 @@ struct xsdfec_dev {
struct device *dev;
enum xsdfec_state state;
struct xsdfec_config config;
+ bool intr_enabled;
bool state_updated;
bool stats_updated;
atomic_t isr_err_count;
@@ -323,6 +325,28 @@ static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
return 0;
}

+static int xsdfec_get_status(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ struct xsdfec_status status;
+ int err;
+
+ status.fec_id = xsdfec->config.fec_id;
+ spin_lock_irq(&xsdfec->irq_lock);
+ status.state = xsdfec->state;
+ xsdfec->state_updated = false;
+ spin_unlock_irq(&xsdfec->irq_lock);
+ status.activity = (xsdfec_regread(xsdfec, XSDFEC_ACTIVE_ADDR) &
+ XSDFEC_IS_ACTIVITY_SET);
+
+ err = copy_to_user(arg, &status, sizeof(status));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ err = -EFAULT;
+ }
+ return err;
+}
+
static int xsdfec_get_config(struct xsdfec_dev *xsdfec, void __user *arg)
{
int err;
@@ -908,6 +932,83 @@ static int xsdfec_cfg_axi_streams(struct xsdfec_dev *xsdfec)
return 0;
}

+static int xsdfec_start(struct xsdfec_dev *xsdfec)
+{
+ u32 regread;
+
+ regread = xsdfec_regread(xsdfec, XSDFEC_FEC_CODE_ADDR);
+ regread &= 0x1;
+ if (regread != xsdfec->config.code) {
+ dev_err(xsdfec->dev,
+ "%s SDFEC HW code does not match driver code, reg %d, code %d",
+ __func__, regread, xsdfec->config.code);
+ return -EINVAL;
+ }
+
+ /* Set AXIS enable */
+ xsdfec_regwrite(xsdfec, XSDFEC_AXIS_ENABLE_ADDR,
+ XSDFEC_AXIS_ENABLE_MASK);
+ /* Done */
+ xsdfec->state = XSDFEC_STARTED;
+ return 0;
+}
+
+static int xsdfec_stop(struct xsdfec_dev *xsdfec)
+{
+ u32 regread;
+
+ if (xsdfec->state != XSDFEC_STARTED)
+ dev_err(xsdfec->dev, "Device not started correctly");
+ /* Disable AXIS_ENABLE Input interfaces only */
+ regread = xsdfec_regread(xsdfec, XSDFEC_AXIS_ENABLE_ADDR);
+ regread &= (~XSDFEC_AXIS_IN_ENABLE_MASK);
+ xsdfec_regwrite(xsdfec, XSDFEC_AXIS_ENABLE_ADDR, regread);
+ /* Stop */
+ xsdfec->state = XSDFEC_STOPPED;
+ return 0;
+}
+
+static int xsdfec_clear_stats(struct xsdfec_dev *xsdfec)
+{
+ atomic_set(&xsdfec->isr_err_count, 0);
+ atomic_set(&xsdfec->uecc_count, 0);
+ atomic_set(&xsdfec->cecc_count, 0);
+
+ return 0;
+}
+
+static int xsdfec_get_stats(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ int err;
+ struct xsdfec_stats user_stats;
+
+ spin_lock_irq(&xsdfec->irq_lock);
+ user_stats.isr_err_count = atomic_read(&xsdfec->isr_err_count);
+ user_stats.cecc_count = atomic_read(&xsdfec->cecc_count);
+ user_stats.uecc_count = atomic_read(&xsdfec->uecc_count);
+ xsdfec->stats_updated = false;
+ spin_unlock_irq(&xsdfec->irq_lock);
+
+ err = copy_to_user(arg, &user_stats, sizeof(user_stats));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ err = -EFAULT;
+ }
+
+ return err;
+}
+
+static int xsdfec_set_default_config(struct xsdfec_dev *xsdfec)
+{
+ /* Ensure registers are aligned with core configuration */
+ xsdfec_regwrite(xsdfec, XSDFEC_FEC_CODE_ADDR, xsdfec->config.code);
+ xsdfec_cfg_axi_streams(xsdfec);
+ update_config_from_hw(xsdfec);
+
+ return 0;
+}
+
static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
unsigned long data)
{
@@ -919,6 +1020,15 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
if (!xsdfec)
return rval;

+ /* In failed state allow only reset and get status IOCTLs */
+ if (xsdfec->state == XSDFEC_NEEDS_RESET &&
+ (cmd != XSDFEC_SET_DEFAULT_CONFIG && cmd != XSDFEC_GET_STATUS &&
+ cmd != XSDFEC_GET_STATS && cmd != XSDFEC_CLEAR_STATS)) {
+ dev_err(xsdfec->dev, "SDFEC%d in failed state. Reset Required",
+ xsdfec->config.fec_id);
+ return -EPERM;
+ }
+
if (_IOC_TYPE(cmd) != XSDFEC_MAGIC) {
dev_err(xsdfec->dev, "Not a xilinx sdfec ioctl");
return -ENOTTY;
@@ -946,9 +1056,27 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
}

switch (cmd) {
+ case XSDFEC_START_DEV:
+ rval = xsdfec_start(xsdfec);
+ break;
+ case XSDFEC_STOP_DEV:
+ rval = xsdfec_stop(xsdfec);
+ break;
+ case XSDFEC_CLEAR_STATS:
+ rval = xsdfec_clear_stats(xsdfec);
+ break;
+ case XSDFEC_GET_STATS:
+ rval = xsdfec_get_stats(xsdfec, arg);
+ break;
+ case XSDFEC_GET_STATUS:
+ rval = xsdfec_get_status(xsdfec, arg);
+ break;
case XSDFEC_GET_CONFIG:
rval = xsdfec_get_config(xsdfec, arg);
break;
+ case XSDFEC_SET_DEFAULT_CONFIG:
+ rval = xsdfec_set_default_config(xsdfec);
+ break;
case XSDFEC_SET_IRQ:
rval = xsdfec_set_irq(xsdfec, arg);
break;
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index 8dfada9..df47995 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -235,6 +235,21 @@ struct xsdfec_config {
};

/**
+ * struct xsdfec_stats - Stats retrived by ioctl XSDFEC_GET_STATS. Used
+ * to buffer atomic_t variables from struct
+ * xsdfec_dev. Counts are accumulated until
+ * the user clears them.
+ * @isr_err_count: Count of ISR errors
+ * @cecc_count: Count of Correctable ECC errors (SBE)
+ * @uecc_count: Count of Uncorrectable ECC errors (MBE)
+ */
+struct xsdfec_stats {
+ u32 isr_err_count;
+ u32 cecc_count;
+ u32 uecc_count;
+};
+
+/**
* struct xsdfec_ldpc_param_table_sizes - Used to store sizes of SD-FEC table
* entries for an individual LPDC code
* parameter.
@@ -274,6 +289,32 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
*/
#define XSDFEC_MAGIC 'f'
/**
+ * DOC: XSDFEC_START_DEV
+ *
+ * @Description
+ *
+ * ioctl to start SD-FEC core
+ *
+ * This fails if the XSDFEC_SET_ORDER ioctl has not been previously called
+ */
+#define XSDFEC_START_DEV _IO(XSDFEC_MAGIC, 0)
+/**
+ * DOC: XSDFEC_STOP_DEV
+ *
+ * @Description
+ *
+ * ioctl to stop the SD-FEC core
+ */
+#define XSDFEC_STOP_DEV _IO(XSDFEC_MAGIC, 1)
+/**
+ * DOC: XSDFEC_GET_STATUS
+ *
+ * @Description
+ *
+ * ioctl that returns status of SD-FEC core
+ */
+#define XSDFEC_GET_STATUS _IOR(XSDFEC_MAGIC, 2, struct xsdfec_status *)
+/**
* DOC: XSDFEC_SET_IRQ
* @Parameters
*
@@ -392,4 +433,38 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
* ioctl that determines if SD-FEC is processing data
*/
#define XSDFEC_IS_ACTIVE _IOR(XSDFEC_MAGIC, 10, bool *)
+/**
+ * DOC: XSDFEC_CLEAR_STATS
+ *
+ * @Description
+ *
+ * ioctl that clears error stats collected during interrupts
+ */
+#define XSDFEC_CLEAR_STATS _IO(XSDFEC_MAGIC, 11)
+/**
+ * DOC: XSDFEC_GET_STATS
+ * @Parameters
+ *
+ * @struct xsdfec_stats *
+ * Pointer to the &struct xsdfec_stats that will contain the updated stats
+ * values
+ *
+ * @Description
+ *
+ * ioctl that returns SD-FEC core stats
+ *
+ * This can only be used when the driver is in the XSDFEC_STOPPED state
+ */
+#define XSDFEC_GET_STATS _IOR(XSDFEC_MAGIC, 12, struct xsdfec_stats *)
+/**
+ * DOC: XSDFEC_SET_DEFAULT_CONFIG
+ *
+ * @Description
+ *
+ * ioctl that returns SD-FEC core to default config, use after a reset
+ *
+ * This can only be used when the driver is in the XSDFEC_STOPPED state
+ */
+#define XSDFEC_SET_DEFAULT_CONFIG _IO(XSDFEC_MAGIC, 13)
+
#endif /* __XILINX_SDFEC_H__ */
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:07:09

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 03/12] misc: xilinx_sdfec: Add CCF support

Add the support for Linux Clock Control Framework (CCF).
Registers and enables clocks with the Clock Control
Framework (CCF), to prevent shared clocks from been
disabled.

Reviewed-by: Michal Simek <[email protected]>
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 | 154 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 154 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index 278754b..a52a5c6 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -37,6 +37,28 @@ static atomic_t xsdfec_ndevs = ATOMIC_INIT(0);
static dev_t xsdfec_devt;

/**
+ * struct xsdfec_clks - For managing SD-FEC clocks
+ * @core_clk: Main processing clock for core
+ * @axi_clk: AXI4-Lite memory-mapped clock
+ * @din_words_clk: DIN Words AXI4-Stream Slave clock
+ * @din_clk: DIN AXI4-Stream Slave clock
+ * @dout_clk: DOUT Words AXI4-Stream Slave clock
+ * @dout_words_clk: DOUT AXI4-Stream Slave clock
+ * @ctrl_clk: Control AXI4-Stream Slave clock
+ * @status_clk: Status AXI4-Stream Slave clock
+ */
+struct xsdfec_clks {
+ struct clk *core_clk;
+ struct clk *axi_clk;
+ struct clk *din_words_clk;
+ struct clk *din_clk;
+ struct clk *dout_clk;
+ struct clk *dout_words_clk;
+ struct clk *ctrl_clk;
+ struct clk *status_clk;
+};
+
+/**
* struct xsdfec_dev - Driver data for SDFEC
* @regs: device physical base address
* @dev: pointer to device struct
@@ -44,6 +66,7 @@ static dev_t xsdfec_devt;
* @open_count: Count of char device being opened
* @xsdfec_cdev: Character device handle
* @irq_lock: Driver spinlock
+ * @clks: Clocks managed by the SDFEC driver
*
* This structure contains necessary state for SDFEC driver to operate
*/
@@ -55,12 +78,136 @@ struct xsdfec_dev {
struct cdev xsdfec_cdev;
/* Spinlock to protect state_updated and stats_updated */
spinlock_t irq_lock;
+ struct xsdfec_clks clks;
};

static const struct file_operations xsdfec_fops = {
.owner = THIS_MODULE,
};

+static int xsdfec_clk_init(struct platform_device *pdev,
+ struct xsdfec_clks *clks)
+{
+ int err;
+
+ clks->core_clk = devm_clk_get(&pdev->dev, "core_clk");
+ if (IS_ERR(clks->core_clk)) {
+ dev_err(&pdev->dev, "failed to get core_clk");
+ return PTR_ERR(clks->core_clk);
+ }
+
+ clks->axi_clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
+ if (IS_ERR(clks->axi_clk)) {
+ dev_err(&pdev->dev, "failed to get axi_clk");
+ return PTR_ERR(clks->axi_clk);
+ }
+
+ clks->din_words_clk = devm_clk_get(&pdev->dev, "s_axis_din_words_aclk");
+ if (IS_ERR(clks->din_words_clk))
+ clks->din_words_clk = NULL;
+
+ clks->din_clk = devm_clk_get(&pdev->dev, "s_axis_din_aclk");
+ if (IS_ERR(clks->din_clk))
+ clks->din_clk = NULL;
+
+ clks->dout_clk = devm_clk_get(&pdev->dev, "m_axis_dout_aclk");
+ if (IS_ERR(clks->dout_clk))
+ clks->dout_clk = NULL;
+
+ clks->dout_words_clk =
+ devm_clk_get(&pdev->dev, "s_axis_dout_words_aclk");
+ if (IS_ERR(clks->dout_words_clk))
+ clks->dout_words_clk = NULL;
+
+ clks->ctrl_clk = devm_clk_get(&pdev->dev, "s_axis_ctrl_aclk");
+ if (IS_ERR(clks->ctrl_clk))
+ clks->ctrl_clk = NULL;
+
+ clks->status_clk = devm_clk_get(&pdev->dev, "m_axis_status_aclk");
+ if (IS_ERR(clks->status_clk))
+ clks->status_clk = NULL;
+
+ err = clk_prepare_enable(clks->core_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable core_clk (%d)", err);
+ return err;
+ }
+
+ err = clk_prepare_enable(clks->axi_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable axi_clk (%d)", err);
+ goto err_disable_core_clk;
+ }
+
+ err = clk_prepare_enable(clks->din_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable din_clk (%d)", err);
+ goto err_disable_axi_clk;
+ }
+
+ err = clk_prepare_enable(clks->din_words_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable din_words_clk (%d)", err);
+ goto err_disable_din_clk;
+ }
+
+ err = clk_prepare_enable(clks->dout_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable dout_clk (%d)", err);
+ goto err_disable_din_words_clk;
+ }
+
+ err = clk_prepare_enable(clks->dout_words_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable dout_words_clk (%d)",
+ err);
+ goto err_disable_dout_clk;
+ }
+
+ err = clk_prepare_enable(clks->ctrl_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable ctrl_clk (%d)", err);
+ goto err_disable_dout_words_clk;
+ }
+
+ err = clk_prepare_enable(clks->status_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable status_clk (%d)\n", err);
+ goto err_disable_ctrl_clk;
+ }
+
+ return err;
+
+err_disable_ctrl_clk:
+ clk_disable_unprepare(clks->ctrl_clk);
+err_disable_dout_words_clk:
+ clk_disable_unprepare(clks->dout_words_clk);
+err_disable_dout_clk:
+ clk_disable_unprepare(clks->dout_clk);
+err_disable_din_words_clk:
+ clk_disable_unprepare(clks->din_words_clk);
+err_disable_din_clk:
+ clk_disable_unprepare(clks->din_clk);
+err_disable_axi_clk:
+ clk_disable_unprepare(clks->axi_clk);
+err_disable_core_clk:
+ clk_disable_unprepare(clks->core_clk);
+
+ return err;
+}
+
+static void xsdfec_disable_all_clks(struct xsdfec_clks *clks)
+{
+ clk_disable_unprepare(clks->status_clk);
+ clk_disable_unprepare(clks->ctrl_clk);
+ clk_disable_unprepare(clks->dout_words_clk);
+ clk_disable_unprepare(clks->dout_clk);
+ clk_disable_unprepare(clks->din_words_clk);
+ clk_disable_unprepare(clks->din_clk);
+ clk_disable_unprepare(clks->core_clk);
+ clk_disable_unprepare(clks->axi_clk);
+}
+
static int xsdfec_probe(struct platform_device *pdev)
{
struct xsdfec_dev *xsdfec;
@@ -77,6 +224,10 @@ static int xsdfec_probe(struct platform_device *pdev)
xsdfec->config.fec_id = atomic_read(&xsdfec_ndevs);
spin_lock_init(&xsdfec->irq_lock);

+ err = xsdfec_clk_init(pdev, &xsdfec->clks);
+ if (err)
+ return err;
+
dev = xsdfec->dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
xsdfec->regs = devm_ioremap_resource(dev, res);
@@ -124,6 +275,7 @@ static int xsdfec_probe(struct platform_device *pdev)
err_xsdfec_cdev:
cdev_del(&xsdfec->xsdfec_cdev);
err_xsdfec_dev:
+ xsdfec_disable_all_clks(&xsdfec->clks);
return err;
}

@@ -141,6 +293,8 @@ static int xsdfec_remove(struct platform_device *pdev)
return -EIO;
}

+ xsdfec_disable_all_clks(&xsdfec->clks);
+
device_destroy(xsdfec_class,
MKDEV(MAJOR(xsdfec_devt), xsdfec->config.fec_id));
cdev_del(&xsdfec->xsdfec_cdev);
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:07:19

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 02/12] misc: xilinx-sdfec: add core driver

Implements an platform driver that matches with xlnx,
sd-fec-1.1 device tree node and registers as a character
device, including:
- SD-FEC driver binds to sdfec DT node.
- creates and initialise an initial driver dev structure.
- add the driver in Linux build and Kconfig.

Reviewed-by: Michal Simek <[email protected]>
Tested-by: Dragan Cvetic <[email protected]>
Signed-off-by: Derek Kiernan <[email protected]>
Signed-off-by: Dragan Cvetic <[email protected]>
---
drivers/misc/Kconfig | 12 +++
drivers/misc/Makefile | 1 +
drivers/misc/xilinx_sdfec.c | 215 +++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 42 ++++++++
4 files changed, 270 insertions(+)
create mode 100644 drivers/misc/xilinx_sdfec.c
create mode 100644 include/uapi/misc/xilinx_sdfec.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 42ab8ec..bbb868f 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -520,6 +520,18 @@ config PCI_ENDPOINT_TEST
Enable this configuration option to enable the host side test driver
for PCI Endpoint.

+config XILINX_SDFEC
+ tristate "Xilinx SDFEC 16"
+ help
+ This option enables support for the Xilinx SDFEC (Soft Decision
+ Forward Error Correction) driver. This enables a char driver
+ for the SDFEC.
+
+ You may select this driver if your design instantiates the
+ SDFEC(16nm) hardened block. To compile this as a module choose M.
+
+ If unsure, say N.
+
config MISC_RTSX
tristate
default MISC_RTSX_PCI || MISC_RTSX_USB
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index d5b7d34..08f0906 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
obj-$(CONFIG_SRAM) += sram.o
obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
+obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
obj-y += mic/
obj-$(CONFIG_GENWQE) += genwqe/
obj-$(CONFIG_ECHO) += echo/
diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
new file mode 100644
index 0000000..278754b
--- /dev/null
+++ b/drivers/misc/xilinx_sdfec.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx SDFEC
+ *
+ * Copyright (C) 2016 - 2017 Xilinx, Inc.
+ *
+ * Description:
+ * This driver is developed for SDFEC16 (Soft Decision FEC 16nm)
+ * IP. It exposes a char device interface in sysfs and supports file
+ * operations like open(), close() and ioctl().
+ */
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/spinlock.h>
+#include <linux/clk.h>
+
+#include <uapi/misc/xilinx_sdfec.h>
+
+#define DRIVER_NAME "xilinx_sdfec"
+#define DRIVER_VERSION "0.3"
+#define DRIVER_MAX_DEV BIT(MINORBITS)
+
+static struct class *xsdfec_class;
+static atomic_t xsdfec_ndevs = ATOMIC_INIT(0);
+static dev_t xsdfec_devt;
+
+/**
+ * struct xsdfec_dev - Driver data for SDFEC
+ * @regs: device physical base address
+ * @dev: pointer to device struct
+ * @config: Configuration of the SDFEC device
+ * @open_count: Count of char device being opened
+ * @xsdfec_cdev: Character device handle
+ * @irq_lock: Driver spinlock
+ *
+ * This structure contains necessary state for SDFEC driver to operate
+ */
+struct xsdfec_dev {
+ void __iomem *regs;
+ struct device *dev;
+ struct xsdfec_config config;
+ atomic_t open_count;
+ struct cdev xsdfec_cdev;
+ /* Spinlock to protect state_updated and stats_updated */
+ spinlock_t irq_lock;
+};
+
+static const struct file_operations xsdfec_fops = {
+ .owner = THIS_MODULE,
+};
+
+static int xsdfec_probe(struct platform_device *pdev)
+{
+ struct xsdfec_dev *xsdfec;
+ struct device *dev;
+ struct device *dev_create;
+ struct resource *res;
+ int err;
+
+ xsdfec = devm_kzalloc(&pdev->dev, sizeof(*xsdfec), GFP_KERNEL);
+ if (!xsdfec)
+ return -ENOMEM;
+
+ xsdfec->dev = &pdev->dev;
+ xsdfec->config.fec_id = atomic_read(&xsdfec_ndevs);
+ spin_lock_init(&xsdfec->irq_lock);
+
+ dev = xsdfec->dev;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ xsdfec->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(xsdfec->regs)) {
+ dev_err(dev, "Unable to map resource");
+ err = PTR_ERR(xsdfec->regs);
+ goto err_xsdfec_dev;
+ }
+
+ /* Save driver private data */
+ platform_set_drvdata(pdev, xsdfec);
+
+ cdev_init(&xsdfec->xsdfec_cdev, &xsdfec_fops);
+ xsdfec->xsdfec_cdev.owner = THIS_MODULE;
+ err = cdev_add(&xsdfec->xsdfec_cdev,
+ MKDEV(MAJOR(xsdfec_devt), xsdfec->config.fec_id), 1);
+ if (err < 0) {
+ dev_err(dev, "cdev_add failed");
+ err = -EIO;
+ goto err_xsdfec_dev;
+ }
+
+ if (!xsdfec_class) {
+ err = -EIO;
+ dev_err(dev, "xsdfec class not created correctly");
+ goto err_xsdfec_cdev;
+ }
+
+ dev_create =
+ device_create(xsdfec_class, dev,
+ MKDEV(MAJOR(xsdfec_devt), xsdfec->config.fec_id),
+ xsdfec, "xsdfec%d", xsdfec->config.fec_id);
+ if (IS_ERR(dev_create)) {
+ dev_err(dev, "unable to create device");
+ err = PTR_ERR(dev_create);
+ goto err_xsdfec_cdev;
+ }
+
+ atomic_set(&xsdfec->open_count, 1);
+ dev_info(dev, "XSDFEC%d Probe Successful", xsdfec->config.fec_id);
+ atomic_inc(&xsdfec_ndevs);
+ return 0;
+
+ /* Failure cleanup */
+err_xsdfec_cdev:
+ cdev_del(&xsdfec->xsdfec_cdev);
+err_xsdfec_dev:
+ return err;
+}
+
+static int xsdfec_remove(struct platform_device *pdev)
+{
+ struct xsdfec_dev *xsdfec;
+ struct device *dev = &pdev->dev;
+
+ xsdfec = platform_get_drvdata(pdev);
+ if (!xsdfec)
+ return -ENODEV;
+
+ if (!xsdfec_class) {
+ dev_err(dev, "xsdfec_class is NULL");
+ return -EIO;
+ }
+
+ device_destroy(xsdfec_class,
+ MKDEV(MAJOR(xsdfec_devt), xsdfec->config.fec_id));
+ cdev_del(&xsdfec->xsdfec_cdev);
+ atomic_dec(&xsdfec_ndevs);
+ return 0;
+}
+
+static const struct of_device_id xsdfec_of_match[] = {
+ {
+ .compatible = "xlnx,sd-fec-1.1",
+ },
+ { /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, xsdfec_of_match);
+
+static struct platform_driver xsdfec_driver = {
+ .driver = {
+ .name = "xilinx-sdfec",
+ .of_match_table = xsdfec_of_match,
+ },
+ .probe = xsdfec_probe,
+ .remove = xsdfec_remove,
+};
+
+static int __init xsdfec_init_mod(void)
+{
+ int err;
+
+ xsdfec_class = class_create(THIS_MODULE, DRIVER_NAME);
+ if (IS_ERR(xsdfec_class)) {
+ err = PTR_ERR(xsdfec_class);
+ pr_err("%s : Unable to register xsdfec class", __func__);
+ return err;
+ }
+
+ err = alloc_chrdev_region(&xsdfec_devt, 0, DRIVER_MAX_DEV, DRIVER_NAME);
+ if (err < 0) {
+ pr_err("%s : Unable to get major number", __func__);
+ goto err_xsdfec_class;
+ }
+
+ err = platform_driver_register(&xsdfec_driver);
+ if (err < 0) {
+ pr_err("%s Unabled to register %s driver", __func__,
+ DRIVER_NAME);
+ goto err_xsdfec_drv;
+ }
+ return 0;
+
+ /* Error Path */
+err_xsdfec_drv:
+ unregister_chrdev_region(xsdfec_devt, DRIVER_MAX_DEV);
+err_xsdfec_class:
+ class_destroy(xsdfec_class);
+ return err;
+}
+
+static void __exit xsdfec_cleanup_mod(void)
+{
+ platform_driver_unregister(&xsdfec_driver);
+ unregister_chrdev_region(xsdfec_devt, DRIVER_MAX_DEV);
+ class_destroy(xsdfec_class);
+ xsdfec_class = NULL;
+}
+
+module_init(xsdfec_init_mod);
+module_exit(xsdfec_cleanup_mod);
+
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Xilinx SD-FEC16 Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRIVER_VERSION);
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
new file mode 100644
index 0000000..5543163
--- /dev/null
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * 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__
+
+/**
+ * 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,
+};
+
+/**
+ * struct xsdfec_config - Configuration of SD-FEC core.
+ * @fec_id: ID of SD-FEC instance. ID is limited to the number of active
+ * SD-FEC's in the FPGA and is related to the driver instance
+ * Minor number.
+ */
+struct xsdfec_config {
+ s32 fec_id;
+};
+
+#endif /* __XILINX_SDFEC_H__ */
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:08:13

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl

Add char device interface per DT node present and support
file operations:
- open(), which keeps only one open per device at a time,
- close(), which release the open for this device,
- ioctl(), which provides infrastructure for a specific driver
control.

Reviewed-by: Michal Simek <[email protected]>
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 | 79 ++++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 4 ++
2 files changed, 83 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index a52a5c6..3407de4 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -81,8 +81,87 @@ struct xsdfec_dev {
struct xsdfec_clks clks;
};

+static int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
+{
+ struct xsdfec_dev *xsdfec;
+
+ xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
+ if (!xsdfec)
+ return -EAGAIN;
+
+ /* Only one open per device at a time */
+ if (!atomic_dec_and_test(&xsdfec->open_count)) {
+ atomic_inc(&xsdfec->open_count);
+ return -EBUSY;
+ }
+
+ fptr->private_data = xsdfec;
+ return 0;
+}
+
+static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
+{
+ struct xsdfec_dev *xsdfec;
+
+ xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
+ if (!xsdfec)
+ return -EAGAIN;
+
+ atomic_inc(&xsdfec->open_count);
+ return 0;
+}
+
+static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
+ unsigned long data)
+{
+ struct xsdfec_dev *xsdfec = fptr->private_data;
+ void __user *arg = NULL;
+ int rval = -EINVAL;
+ int err = 0;
+
+ if (!xsdfec)
+ return rval;
+
+ if (_IOC_TYPE(cmd) != XSDFEC_MAGIC) {
+ dev_err(xsdfec->dev, "Not a xilinx sdfec ioctl");
+ return -ENOTTY;
+ }
+
+ /* check if ioctl argument is present and valid */
+ if (_IOC_DIR(cmd) != _IOC_NONE) {
+ arg = (void __user *)data;
+ if (!arg) {
+ dev_err(xsdfec->dev,
+ "xilinx sdfec ioctl argument is NULL Pointer");
+ return rval;
+ }
+ }
+
+ /* Access check of the argument if present */
+ if (_IOC_DIR(cmd) & _IOC_READ)
+ err = !access_ok((void *)arg, _IOC_SIZE(cmd));
+ else if (_IOC_DIR(cmd) & _IOC_WRITE)
+ err = !access_ok((void *)arg, _IOC_SIZE(cmd));
+
+ if (err) {
+ dev_err(xsdfec->dev, "Invalid xilinx sdfec ioctl argument");
+ return -EFAULT;
+ }
+
+ switch (cmd) {
+ default:
+ /* Should not get here */
+ dev_err(xsdfec->dev, "Undefined SDFEC IOCTL");
+ break;
+ }
+ return rval;
+}
+
static const struct file_operations xsdfec_fops = {
.owner = THIS_MODULE,
+ .open = xsdfec_dev_open,
+ .release = xsdfec_dev_release,
+ .unlocked_ioctl = xsdfec_dev_ioctl,
};

static int xsdfec_clk_init(struct platform_device *pdev,
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index 5543163..5de0add 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -39,4 +39,8 @@ struct xsdfec_config {
s32 fec_id;
};

+/*
+ * XSDFEC IOCTL List
+ */
+#define XSDFEC_MAGIC 'f'
#endif /* __XILINX_SDFEC_H__ */
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:08:14

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 12/12] MAINTAINERS: add maintainer for SD-FEC support

Add maintainer entry for Xilinx SD-FEC driver support.

Signed-off-by: Derek Kiernan <[email protected]>
Signed-off-by: Dragan Cvetic <[email protected]>
---
MAINTAINERS | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e17ebf7..ba7a0be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17093,6 +17093,18 @@ S: Supported
F: Documentation/filesystems/xfs.txt
F: fs/xfs/

+XILINX SD-FEC IP CORES
+M: Derek Kiernan <[email protected]>
+M: Dragan Cvetic <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/misc/xlnx,sd-fec.txt
+F: Documentation/misc-devices/xilinx_sdfec.rst
+F: Documentation/misc-devices/index.rst
+F: drivers/misc/xilinx_sdfec.c
+F: drivers/misc/Kconfig
+F: drivers/misc/Makefile
+F: include/uapi/misc/xilinx_sdfec.h
+
XILINX AXI ETHERNET DRIVER
M: Anirudha Sarangi <[email protected]>
M: John Linn <[email protected]>
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:08:28

by Dragan Cvetic

[permalink] [raw]
Subject: [PATCH 06/12] misc: xilinx_sdfec: Add ability to configure turbo mode

Add the capability to configure and retrieve turbo mode
via the ioctls XSDFEC_SET_TURBO and XSDFEC_GET_TURBO.

Reviewed-by: Michal Simek <[email protected]>
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 | 83 ++++++++++++++++++++++++++++++++++++++++
include/uapi/misc/xilinx_sdfec.h | 71 ++++++++++++++++++++++++++++++++++
2 files changed, 154 insertions(+)

diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
index d9ead0b..58b1c57 100644
--- a/drivers/misc/xilinx_sdfec.c
+++ b/drivers/misc/xilinx_sdfec.c
@@ -115,6 +115,12 @@ static dev_t xsdfec_devt;
/* BYPASS Register */
#define XSDFEC_BYPASS_ADDR (0x3C)

+/* Turbo Code Register */
+#define XSDFEC_TURBO_ADDR (0x100)
+#define XSDFEC_TURBO_SCALE_MASK (0xFFF)
+#define XSDFEC_TURBO_SCALE_BIT_POS (8)
+#define XSDFEC_TURBO_SCALE_MAX (15)
+
/**
* struct xsdfec_clks - For managing SD-FEC clocks
* @core_clk: Main processing clock for core
@@ -251,6 +257,77 @@ static int xsdfec_dev_release(struct inode *iptr, struct file *fptr)
return 0;
}

+static int xsdfec_set_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ struct xsdfec_turbo turbo;
+ int err;
+ u32 turbo_write;
+
+ err = copy_from_user(&turbo, arg, sizeof(turbo));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ return -EFAULT;
+ }
+
+ if (turbo.alg >= XSDFEC_TURBO_ALG_MAX) {
+ dev_err(xsdfec->dev,
+ "%s invalid turbo alg value %d for SDFEC%d", __func__,
+ turbo.alg, xsdfec->config.fec_id);
+ return -EINVAL;
+ }
+
+ if (turbo.scale > XSDFEC_TURBO_SCALE_MAX) {
+ dev_err(xsdfec->dev,
+ "%s invalid turbo scale value %d for SDFEC%d", __func__,
+ turbo.scale, xsdfec->config.fec_id);
+ return -EINVAL;
+ }
+
+ /* Check to see what device tree says about the FEC codes */
+ if (xsdfec->config.code == XSDFEC_LDPC_CODE) {
+ dev_err(xsdfec->dev,
+ "%s: Unable to write Turbo to SDFEC%d check DT",
+ __func__, xsdfec->config.fec_id);
+ return -EIO;
+ }
+
+ turbo_write = ((turbo.scale & XSDFEC_TURBO_SCALE_MASK)
+ << XSDFEC_TURBO_SCALE_BIT_POS) |
+ turbo.alg;
+ xsdfec_regwrite(xsdfec, XSDFEC_TURBO_ADDR, turbo_write);
+ return err;
+}
+
+static int xsdfec_get_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
+{
+ u32 reg_value;
+ struct xsdfec_turbo turbo_params;
+ int err;
+
+ if (xsdfec->config.code == XSDFEC_LDPC_CODE) {
+ dev_err(xsdfec->dev,
+ "%s: SDFEC%d is configured for LDPC, check DT",
+ __func__, xsdfec->config.fec_id);
+ return -EIO;
+ }
+
+ reg_value = xsdfec_regread(xsdfec, XSDFEC_TURBO_ADDR);
+
+ turbo_params.scale = (reg_value & XSDFEC_TURBO_SCALE_MASK) >>
+ XSDFEC_TURBO_SCALE_BIT_POS;
+ turbo_params.alg = reg_value & 0x1;
+
+ err = copy_to_user(arg, &turbo_params, sizeof(turbo_params));
+ if (err) {
+ dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
+ xsdfec->config.fec_id);
+ err = -EFAULT;
+ }
+
+ return err;
+}
+
static u32
xsdfec_translate_axis_width_cfg_val(enum xsdfec_axis_width axis_width_cfg)
{
@@ -352,6 +429,12 @@ static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
}

switch (cmd) {
+ case XSDFEC_SET_TURBO:
+ rval = xsdfec_set_turbo(xsdfec, arg);
+ break;
+ case XSDFEC_GET_TURBO:
+ rval = xsdfec_get_turbo(xsdfec, arg);
+ break;
default:
/* Should not get here */
dev_err(xsdfec->dev, "Undefined SDFEC IOCTL");
diff --git a/include/uapi/misc/xilinx_sdfec.h b/include/uapi/misc/xilinx_sdfec.h
index 0497b66..1a15771 100644
--- a/include/uapi/misc/xilinx_sdfec.h
+++ b/include/uapi/misc/xilinx_sdfec.h
@@ -39,6 +39,22 @@ enum xsdfec_order {
};

/**
+ * enum xsdfec_turbo_alg - Turbo Algorithm Type.
+ * @XSDFEC_MAX_SCALE: Max Log-Map algorithm with extrinsic scaling. When
+ * scaling is set to this is equivalent to the Max Log-Map
+ * algorithm.
+ * @XSDFEC_MAX_STAR: Log-Map algorithm.
+ * @XSDFEC_TURBO_ALG_MAX: Used to indicate out of bound Turbo algorithms.
+ *
+ * This enum specifies which Turbo Decode algorithm is in use.
+ */
+enum xsdfec_turbo_alg {
+ XSDFEC_MAX_SCALE = 0,
+ XSDFEC_MAX_STAR,
+ XSDFEC_TURBO_ALG_MAX,
+};
+
+/**
* enum xsdfec_state - State.
* @XSDFEC_INIT: Driver is initialized.
* @XSDFEC_STARTED: Driver is started.
@@ -96,6 +112,33 @@ enum xsdfec_axis_word_include {
};

/**
+ * struct xsdfec_turbo - User data for Turbo codes.
+ * @alg: Specifies which Turbo decode algorithm to use
+ * @scale: Specifies the extrinsic scaling to apply when the Max Scale algorithm
+ * has been selected
+ *
+ * Turbo code structure to communicate parameters to XSDFEC driver.
+ */
+struct xsdfec_turbo {
+ enum xsdfec_turbo_alg alg;
+ u8 scale;
+};
+
+/**
+ * struct xsdfec_status - Status of SD-FEC core.
+ * @fec_id: ID of SD-FEC instance. ID is limited to the number of active
+ * SD-FEC's in the FPGA and is related to the driver instance
+ * Minor number.
+ * @state: State of the SD-FEC core
+ * @activity: Describes if the SD-FEC instance is Active
+ */
+struct xsdfec_status {
+ s32 fec_id;
+ enum xsdfec_state state;
+ char activity;
+};
+
+/**
* struct xsdfec_irq - Enabling or Disabling Interrupts.
* @enable_isr: If true enables the ISR
* @enable_ecc_isr: If true enables the ECC ISR
@@ -137,4 +180,32 @@ struct xsdfec_config {
* XSDFEC IOCTL List
*/
#define XSDFEC_MAGIC 'f'
+/**
+ * DOC: XSDFEC_SET_TURBO
+ * @Parameters
+ *
+ * @struct xsdfec_turbo *
+ * Pointer to the &struct xsdfec_turbo that contains the Turbo decode
+ * settings for the SD-FEC core
+ *
+ * @Description
+ *
+ * ioctl that sets the SD-FEC Turbo parameter values
+ *
+ * This can only be used when the driver is in the XSDFEC_STOPPED state
+ */
+#define XSDFEC_SET_TURBO _IOW(XSDFEC_MAGIC, 4, struct xsdfec_turbo *)
+/**
+ * DOC: XSDFEC_GET_TURBO
+ * @Parameters
+ *
+ * @struct xsdfec_turbo *
+ * Pointer to the &struct xsdfec_turbo that contains the current Turbo
+ * decode settings of the SD-FEC Block
+ *
+ * @Description
+ *
+ * ioctl that returns SD-FEC turbo param values
+ */
+#define XSDFEC_GET_TURBO _IOR(XSDFEC_MAGIC, 7, struct xsdfec_turbo *)
#endif /* __XILINX_SDFEC_H__ */
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

2019-03-19 12:19:04

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver

On 19. 03. 19 13:04, Dragan Cvetic wrote:
> This patchset is adding the full Soft Decision Forward Error
> Correction (SD-FEC) driver implementation, driver DT binding and
> driver documentation.
>
> Forward Error Correction (FEC) codes such as Low Density Parity
> Check (LDPC) and turbo codes provide a means to control errors in
> data transmissions over unreliable or noisy communication
> channels. The SD-FEC Integrated Block is an optimized block for
> soft-decision decoding of these codes. Fixed turbo codes are
> supported directly, whereas custom and standardized LDPC codes
> are supported through the ability to specify the parity check
> matrix through an AXI4-Lite bus or using the optional programmable
> (PL)-based support logic. For the further information see
> https://www.xilinx.com/support/documentation/ip_documentation/
> sd_fec/v1_1/pg256-sdfec-integrated-block.pdf
>
> This driver is a platform device driver which supports SDFEC16
> (16nm) IP. SD-FEC driver supports LDPC decoding and encoding and
> Turbo code decoding. LDPC codes can be specified on
> a codeword-by-codeword basis, also a custom LDPC code can be used.
>
> The SD-FEC driver exposes a char device interface and supports
> file operations: open(), close(), poll() and ioctl(). The driver
> allows only one usage of the device, open() limits the number of
> driver instances. The driver also utilize Common Clock Framework
> (CCF).
>
> The control and monitoring is supported over ioctl system call.
> The features supported by ioctl():
> - enable or disable data pipes to/from device
> - configure the FEC algorithm parameters
> - set the order of data
> - provide a control of a SDFEC bypass option
> - activates/deactivates SD-FEC
> - collect and provide statistical data
> - enable/disable interrupt mode
>
> Poll can be utilized to detect errors on IRQ trigger rather than
> using looping status and stats ioctl's.
>
> Reviewed-by: Michal Simek <[email protected]>

I am not using this tag a lot that's why I am curious where you got it
from? I can't see it in Xilinx tree too.

Thanks,
Michal

2019-03-19 13:09:19

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 08/12] misc: xilinx_sdfec: Add ability to get/set config

On Tue, Mar 19, 2019 at 1:06 PM Dragan Cvetic <[email protected]> wrote:
> - Add capability to get SD-FEC config data using ioctl
> XSDFEC_GET_CONFIG.
>
> - Add capability to set SD-FEC data order using ioctl
> SDFEC_SET_ORDER. The order of data blocks can change
> from input to output or to be maintained.

Commenting here only on the ABI, not the actual behavior of the driver:

> +static int xsdfec_get_config(struct xsdfec_dev *xsdfec, void __user *arg)
> +{
> + int err;
> +
> + err = copy_to_user(arg, &xsdfec->config, sizeof(xsdfec->config));
> + if (err) {
> + dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
> + xsdfec->config.fec_id);
> + err = -EFAULT;
> + }

Try to avoid printing error messages for things that can be triggered from
user space.

> static int xsdfec_set_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
> {
> struct xsdfec_turbo turbo;
> @@ -670,6 +683,67 @@ static int xsdfec_add_ldpc(struct xsdfec_dev *xsdfec, void __user *arg)
> return ret;
> }
>
> +static int xsdfec_set_order(struct xsdfec_dev *xsdfec, void __user *arg)
> +{
> + bool order_invalid;
> + enum xsdfec_order order = *((enum xsdfec_order *)arg);

Generally speaking, you should never cast between __user pointers
and kernel pointers. It looks like this will actually dereference a
__user pointer, which is a security issue.

Another problem is that the command is defined as

#define XSDFEC_SET_ORDER _IOW(XSDFEC_MAGIC, 8, unsigned long *)

which would indicate an argument of type 'unsigned long __user * __user *',
which is incompatible with 'enum xsdfec_order __user *'. Both enum
and pointer types are variable length and should be avoided in
ioctl commands. Best make this a '__u64 __user *' or '__u32 __user *'.

> + */
> +#define XSDFEC_GET_CONFIG _IOR(XSDFEC_MAGIC, 6, struct xsdfec_config *)
> +/**
> * DOC: XSDFEC_GET_TURBO
> * @Parameters
> *
> @@ -322,4 +335,48 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
> * ioctl that returns SD-FEC turbo param values
> */
> #define XSDFEC_GET_TURBO _IOR(XSDFEC_MAGIC, 7, struct xsdfec_turbo *)

Also wrong type, the function takes a 'struct xsdfec_turbo __user *', not a
'struct xsdfec_turbo __user * __user *',

Arnd

2019-03-19 13:18:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl

On Tue, Mar 19, 2019 at 1:05 PM Dragan Cvetic <[email protected]> wrote:
>
> Add char device interface per DT node present and support
> file operations:
> - open(), which keeps only one open per device at a time,
> - close(), which release the open for this device,
> - ioctl(), which provides infrastructure for a specific driver
> control.

> drivers/misc/xilinx_sdfec.c | 79 ++++++++++++++++++++++++++++++++++++++++
> include/uapi/misc/xilinx_sdfec.h | 4 ++
> 2 files changed, 83 insertions(+)
>
> diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> index a52a5c6..3407de4 100644
> --- a/drivers/misc/xilinx_sdfec.c
> +++ b/drivers/misc/xilinx_sdfec.c
> @@ -81,8 +81,87 @@ struct xsdfec_dev {
> struct xsdfec_clks clks;
> };
>
> +static int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
> +{
> + struct xsdfec_dev *xsdfec;
> +
> + xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
> + if (!xsdfec)
> + return -EAGAIN;

The result of container_of() will not be NULL here.
Did you mean to check i_cdev? That probably also won't
be NULL, but that check would be more reasonable.

> + /* Only one open per device at a time */
> + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> + atomic_inc(&xsdfec->open_count);
> + return -EBUSY;
> + }

What is that limitation for? Is it worse to open it twice than
to dup() or fork()?

Note that the test is not really atomic either: if three processes
try to open the file at the same time, it gets decremented from
1 to -2, so only the second one sees 0 and increments it back
to -1 afterwards...

> +static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
> + unsigned long data)
> +{
> + struct xsdfec_dev *xsdfec = fptr->private_data;
> + void __user *arg = NULL;
> + int rval = -EINVAL;
> + int err = 0;
> +
> + if (!xsdfec)
> + return rval;
> +
> + if (_IOC_TYPE(cmd) != XSDFEC_MAGIC) {
> + dev_err(xsdfec->dev, "Not a xilinx sdfec ioctl");
> + return -ENOTTY;
> + }

remove the error messages here as well.

> + /* Access check of the argument if present */
> + if (_IOC_DIR(cmd) & _IOC_READ)
> + err = !access_ok((void *)arg, _IOC_SIZE(cmd));
> + else if (_IOC_DIR(cmd) & _IOC_WRITE)
> + err = !access_ok((void *)arg, _IOC_SIZE(cmd));

This seems odd. Why two separate checks, and why the access_ok()
check when you do a copy_from_user() that contains the same check
later?

If you want to get fancy here, you could just copy the data in the main
ioctl handler based on _IOC_SIZE, and pass around normal kernel
pointers from there.

> static const struct file_operations xsdfec_fops = {
> .owner = THIS_MODULE,
> + .open = xsdfec_dev_open,
> + .release = xsdfec_dev_release,
> + .unlocked_ioctl = xsdfec_dev_ioctl,
> };

This lacks a .compat_ioctl pointer.

Arnd

2019-03-19 13:47:44

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 00/12] misc: xilinx sd-fec driver



> -----Original Message-----
> From: Michal Simek [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 12:18
> To: Dragan Cvetic <[email protected]>; [email protected]; [email protected]; Michal Simek <[email protected]>; linux-
> [email protected]
> Cc: [email protected]; Derek Kiernan <[email protected]>
> Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver
>
> On 19. 03. 19 13:04, Dragan Cvetic wrote:
> > This patchset is adding the full Soft Decision Forward Error
> > Correction (SD-FEC) driver implementation, driver DT binding and
> > driver documentation.
> >
> > Forward Error Correction (FEC) codes such as Low Density Parity
> > Check (LDPC) and turbo codes provide a means to control errors in
> > data transmissions over unreliable or noisy communication
> > channels. The SD-FEC Integrated Block is an optimized block for
> > soft-decision decoding of these codes. Fixed turbo codes are
> > supported directly, whereas custom and standardized LDPC codes
> > are supported through the ability to specify the parity check
> > matrix through an AXI4-Lite bus or using the optional programmable
> > (PL)-based support logic. For the further information see
> > https://www.xilinx.com/support/documentation/ip_documentation/
> > sd_fec/v1_1/pg256-sdfec-integrated-block.pdf
> >
> > This driver is a platform device driver which supports SDFEC16
> > (16nm) IP. SD-FEC driver supports LDPC decoding and encoding and
> > Turbo code decoding. LDPC codes can be specified on
> > a codeword-by-codeword basis, also a custom LDPC code can be used.
> >
> > The SD-FEC driver exposes a char device interface and supports
> > file operations: open(), close(), poll() and ioctl(). The driver
> > allows only one usage of the device, open() limits the number of
> > driver instances. The driver also utilize Common Clock Framework
> > (CCF).
> >
> > The control and monitoring is supported over ioctl system call.
> > The features supported by ioctl():
> > - enable or disable data pipes to/from device
> > - configure the FEC algorithm parameters
> > - set the order of data
> > - provide a control of a SDFEC bypass option
> > - activates/deactivates SD-FEC
> > - collect and provide statistical data
> > - enable/disable interrupt mode
> >
> > Poll can be utilized to detect errors on IRQ trigger rather than
> > using looping status and stats ioctl's.
> >
> > Reviewed-by: Michal Simek <[email protected]>
>
> I am not using this tag a lot that's why I am curious where you got it
> from? I can't see it in Xilinx tree too.
>
> Thanks,
> Michal

It is copied from the email address in Outlook. I'll correct this to [email protected].

Thanks
Dragan

2019-03-19 13:52:38

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver

On 19. 03. 19 14:44, Dragan Cvetic wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek [mailto:[email protected]]
>> Sent: Tuesday 19 March 2019 12:18
>> To: Dragan Cvetic <[email protected]>; [email protected]; [email protected]; Michal Simek <[email protected]>; linux-
>> [email protected]
>> Cc: [email protected]; Derek Kiernan <[email protected]>
>> Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver
>>
>> On 19. 03. 19 13:04, Dragan Cvetic wrote:
>>> This patchset is adding the full Soft Decision Forward Error
>>> Correction (SD-FEC) driver implementation, driver DT binding and
>>> driver documentation.
>>>
>>> Forward Error Correction (FEC) codes such as Low Density Parity
>>> Check (LDPC) and turbo codes provide a means to control errors in
>>> data transmissions over unreliable or noisy communication
>>> channels. The SD-FEC Integrated Block is an optimized block for
>>> soft-decision decoding of these codes. Fixed turbo codes are
>>> supported directly, whereas custom and standardized LDPC codes
>>> are supported through the ability to specify the parity check
>>> matrix through an AXI4-Lite bus or using the optional programmable
>>> (PL)-based support logic. For the further information see
>>> https://www.xilinx.com/support/documentation/ip_documentation/
>>> sd_fec/v1_1/pg256-sdfec-integrated-block.pdf
>>>
>>> This driver is a platform device driver which supports SDFEC16
>>> (16nm) IP. SD-FEC driver supports LDPC decoding and encoding and
>>> Turbo code decoding. LDPC codes can be specified on
>>> a codeword-by-codeword basis, also a custom LDPC code can be used.
>>>
>>> The SD-FEC driver exposes a char device interface and supports
>>> file operations: open(), close(), poll() and ioctl(). The driver
>>> allows only one usage of the device, open() limits the number of
>>> driver instances. The driver also utilize Common Clock Framework
>>> (CCF).
>>>
>>> The control and monitoring is supported over ioctl system call.
>>> The features supported by ioctl():
>>> - enable or disable data pipes to/from device
>>> - configure the FEC algorithm parameters
>>> - set the order of data
>>> - provide a control of a SDFEC bypass option
>>> - activates/deactivates SD-FEC
>>> - collect and provide statistical data
>>> - enable/disable interrupt mode
>>>
>>> Poll can be utilized to detect errors on IRQ trigger rather than
>>> using looping status and stats ioctl's.
>>>
>>> Reviewed-by: Michal Simek <[email protected]>
>>
>> I am not using this tag a lot that's why I am curious where you got it
>> from? I can't see it in Xilinx tree too.
>>
>> Thanks,
>> Michal
>
> It is copied from the email address in Outlook. I'll correct this to [email protected].

It is not about email address. It is about meaning of that line.
I have never done deep review of this driver that you can put here my
tag that I have reviewed that.
It means you should remove all these lines from all patches were you put
it because I have never gave you this tag.

Thanks,
Michal

2019-03-19 13:57:45

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 00/12] misc: xilinx sd-fec driver



> -----Original Message-----
> From: Michal Simek [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 13:51
> To: Dragan Cvetic <[email protected]>; [email protected]; [email protected]; [email protected]
> Cc: [email protected]; Derek Kiernan <[email protected]>
> Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver
>
> On 19. 03. 19 14:44, Dragan Cvetic wrote:
> >
> >
> >> -----Original Message-----
> >> From: Michal Simek [mailto:[email protected]]
> >> Sent: Tuesday 19 March 2019 12:18
> >> To: Dragan Cvetic <[email protected]>; [email protected]; [email protected]; Michal Simek <[email protected]>;
> linux-
> >> [email protected]
> >> Cc: [email protected]; Derek Kiernan <[email protected]>
> >> Subject: Re: [PATCH 00/12] misc: xilinx sd-fec driver
> >>
> >> On 19. 03. 19 13:04, Dragan Cvetic wrote:
> >>> This patchset is adding the full Soft Decision Forward Error
> >>> Correction (SD-FEC) driver implementation, driver DT binding and
> >>> driver documentation.
> >>>
> >>> Forward Error Correction (FEC) codes such as Low Density Parity
> >>> Check (LDPC) and turbo codes provide a means to control errors in
> >>> data transmissions over unreliable or noisy communication
> >>> channels. The SD-FEC Integrated Block is an optimized block for
> >>> soft-decision decoding of these codes. Fixed turbo codes are
> >>> supported directly, whereas custom and standardized LDPC codes
> >>> are supported through the ability to specify the parity check
> >>> matrix through an AXI4-Lite bus or using the optional programmable
> >>> (PL)-based support logic. For the further information see
> >>> https://www.xilinx.com/support/documentation/ip_documentation/
> >>> sd_fec/v1_1/pg256-sdfec-integrated-block.pdf
> >>>
> >>> This driver is a platform device driver which supports SDFEC16
> >>> (16nm) IP. SD-FEC driver supports LDPC decoding and encoding and
> >>> Turbo code decoding. LDPC codes can be specified on
> >>> a codeword-by-codeword basis, also a custom LDPC code can be used.
> >>>
> >>> The SD-FEC driver exposes a char device interface and supports
> >>> file operations: open(), close(), poll() and ioctl(). The driver
> >>> allows only one usage of the device, open() limits the number of
> >>> driver instances. The driver also utilize Common Clock Framework
> >>> (CCF).
> >>>
> >>> The control and monitoring is supported over ioctl system call.
> >>> The features supported by ioctl():
> >>> - enable or disable data pipes to/from device
> >>> - configure the FEC algorithm parameters
> >>> - set the order of data
> >>> - provide a control of a SDFEC bypass option
> >>> - activates/deactivates SD-FEC
> >>> - collect and provide statistical data
> >>> - enable/disable interrupt mode
> >>>
> >>> Poll can be utilized to detect errors on IRQ trigger rather than
> >>> using looping status and stats ioctl's.
> >>>
> >>> Reviewed-by: Michal Simek <[email protected]>
> >>
> >> I am not using this tag a lot that's why I am curious where you got it
> >> from? I can't see it in Xilinx tree too.
> >>
> >> Thanks,
> >> Michal
> >
> > It is copied from the email address in Outlook. I'll correct this to [email protected].
>
> It is not about email address. It is about meaning of that line.
> I have never done deep review of this driver that you can put here my
> tag that I have reviewed that.
> It means you should remove all these lines from all patches were you put
> it because I have never gave you this tag.
>
> Thanks,
> Michal

I'll remove your name.

2019-03-19 14:03:35

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 08/12] misc: xilinx_sdfec: Add ability to get/set config



> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 13:06
> To: Dragan Cvetic <[email protected]>
> Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Linux ARM <[email protected]>;
> Linux Kernel Mailing List <[email protected]>; Derek Kiernan <[email protected]>
> Subject: Re: [PATCH 08/12] misc: xilinx_sdfec: Add ability to get/set config
>
> On Tue, Mar 19, 2019 at 1:06 PM Dragan Cvetic <[email protected]> wrote:
> > - Add capability to get SD-FEC config data using ioctl
> > XSDFEC_GET_CONFIG.
> >
> > - Add capability to set SD-FEC data order using ioctl
> > SDFEC_SET_ORDER. The order of data blocks can change
> > from input to output or to be maintained.
>
> Commenting here only on the ABI, not the actual behavior of the driver:

Will be removed.

>
> > +static int xsdfec_get_config(struct xsdfec_dev *xsdfec, void __user *arg)
> > +{
> > + int err;
> > +
> > + err = copy_to_user(arg, &xsdfec->config, sizeof(xsdfec->config));
> > + if (err) {
> > + dev_err(xsdfec->dev, "%s failed for SDFEC%d", __func__,
> > + xsdfec->config.fec_id);
> > + err = -EFAULT;
> > + }
>
> Try to avoid printing error messages for things that can be triggered from
> user space.

Will be removed.

>
> > static int xsdfec_set_turbo(struct xsdfec_dev *xsdfec, void __user *arg)
> > {
> > struct xsdfec_turbo turbo;
> > @@ -670,6 +683,67 @@ static int xsdfec_add_ldpc(struct xsdfec_dev *xsdfec, void __user *arg)
> > return ret;
> > }
> >
> > +static int xsdfec_set_order(struct xsdfec_dev *xsdfec, void __user *arg)
> > +{
> > + bool order_invalid;
> > + enum xsdfec_order order = *((enum xsdfec_order *)arg);
>
> Generally speaking, you should never cast between __user pointers
> and kernel pointers. It looks like this will actually dereference a
> __user pointer, which is a security issue.
>
> Another problem is that the command is defined as
>
> #define XSDFEC_SET_ORDER _IOW(XSDFEC_MAGIC, 8, unsigned long *)
>
> which would indicate an argument of type 'unsigned long __user * __user *',
> which is incompatible with 'enum xsdfec_order __user *'. Both enum
> and pointer types are variable length and should be avoided in
> ioctl commands. Best make this a '__u64 __user *' or '__u32 __user *'.

Will be updated as proposed.

>
> > + */
> > +#define XSDFEC_GET_CONFIG _IOR(XSDFEC_MAGIC, 6, struct xsdfec_config *)
> > +/**
> > * DOC: XSDFEC_GET_TURBO
> > * @Parameters
> > *
> > @@ -322,4 +335,48 @@ xsdfec_calculate_shared_ldpc_table_entry_size(struct xsdfec_ldpc_params *ldpc,
> > * ioctl that returns SD-FEC turbo param values
> > */
> > #define XSDFEC_GET_TURBO _IOR(XSDFEC_MAGIC, 7, struct xsdfec_turbo *)
>
> Also wrong type, the function takes a 'struct xsdfec_turbo __user *', not a
> 'struct xsdfec_turbo __user * __user *',

Will be updated as proposed

>
> Arnd

Thank you
Dragan

2019-03-19 15:00:51

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl



> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 13:18
> To: Dragan Cvetic <[email protected]>
> Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Linux ARM <[email protected]>;
> Derek Kiernan <[email protected]>; Linux Kernel Mailing List <[email protected]>
> Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl
>
> On Tue, Mar 19, 2019 at 1:05 PM Dragan Cvetic <[email protected]> wrote:
> >
> > Add char device interface per DT node present and support
> > file operations:
> > - open(), which keeps only one open per device at a time,
> > - close(), which release the open for this device,
> > - ioctl(), which provides infrastructure for a specific driver
> > control.
>
> > drivers/misc/xilinx_sdfec.c | 79 ++++++++++++++++++++++++++++++++++++++++
> > include/uapi/misc/xilinx_sdfec.h | 4 ++
> > 2 files changed, 83 insertions(+)
> >
> > diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> > index a52a5c6..3407de4 100644
> > --- a/drivers/misc/xilinx_sdfec.c
> > +++ b/drivers/misc/xilinx_sdfec.c
> > @@ -81,8 +81,87 @@ struct xsdfec_dev {
> > struct xsdfec_clks clks;
> > };
> >
> > +static int xsdfec_dev_open(struct inode *iptr, struct file *fptr)
> > +{
> > + struct xsdfec_dev *xsdfec;
> > +
> > + xsdfec = container_of(iptr->i_cdev, struct xsdfec_dev, xsdfec_cdev);
> > + if (!xsdfec)
> > + return -EAGAIN;
>
> The result of container_of() will not be NULL here.
> Did you mean to check i_cdev? That probably also won't
> be NULL, but that check would be more reasonable.


Will be either removed fully or changed with i_cdev check

>
> > + /* Only one open per device at a time */
> > + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> > + atomic_inc(&xsdfec->open_count);
> > + return -EBUSY;
> > + }
>
> What is that limitation for? Is it worse to open it twice than
> to dup() or fork()?
>
The device can be opened only once.

> Note that the test is not really atomic either: if three processes
> try to open the file at the same time, it gets decremented from
> 1 to -2, so only the second one sees 0 and increments it back
> to -1 afterwards...

It looks you are right. Will fix this. Thank you for the catch.

>
> > +static long xsdfec_dev_ioctl(struct file *fptr, unsigned int cmd,
> > + unsigned long data)
> > +{
> > + struct xsdfec_dev *xsdfec = fptr->private_data;
> > + void __user *arg = NULL;
> > + int rval = -EINVAL;
> > + int err = 0;
> > +
> > + if (!xsdfec)
> > + return rval;
> > +
> > + if (_IOC_TYPE(cmd) != XSDFEC_MAGIC) {
> > + dev_err(xsdfec->dev, "Not a xilinx sdfec ioctl");
> > + return -ENOTTY;
> > + }
>
> remove the error messages here as well.
>
> > + /* Access check of the argument if present */
> > + if (_IOC_DIR(cmd) & _IOC_READ)
> > + err = !access_ok((void *)arg, _IOC_SIZE(cmd));
> > + else if (_IOC_DIR(cmd) & _IOC_WRITE)
> > + err = !access_ok((void *)arg, _IOC_SIZE(cmd));
>
> This seems odd. Why two separate checks, and why the access_ok()
> check when you do a copy_from_user() that contains the same check
> later?

Accepted, will remove it.

>
> If you want to get fancy here, you could just copy the data in the main
> ioctl handler based on _IOC_SIZE, and pass around normal kernel
> pointers from there.


Will not be fancy. Thank you for the advice.

>
> > static const struct file_operations xsdfec_fops = {
> > .owner = THIS_MODULE,
> > + .open = xsdfec_dev_open,
> > + .release = xsdfec_dev_release,
> > + .unlocked_ioctl = xsdfec_dev_ioctl,
> > };
>
> This lacks a .compat_ioctl pointer.

This is new for me, I have to investigate more and propose a solution.
Thank you for suggestion.

>
> Arnd

2019-03-19 15:37:38

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl

On Tue, Mar 19, 2019 at 3:59 PM Dragan Cvetic <[email protected]> wrote:
> >
> > > + /* Only one open per device at a time */
> > > + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> > > + atomic_inc(&xsdfec->open_count);
> > > + return -EBUSY;
> > > + }
> >
> > What is that limitation for? Is it worse to open it twice than
> > to dup() or fork()?
> >
> The device can be opened only once.

What I mean here is that preventing the double open() is
a fairly weak protection: it means you cannot have multiple
'struct file' pointers attached to the same inode, but you
can still have the same 'struct file' being available to
multiple processes.

Arnd

2019-03-19 18:12:00

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl



> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 15:36
> To: Dragan Cvetic <[email protected]>
> Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Linux ARM <[email protected]>;
> Derek Kiernan <[email protected]>; Linux Kernel Mailing List <[email protected]>
> Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl
>
> On Tue, Mar 19, 2019 at 3:59 PM Dragan Cvetic <[email protected]> wrote:
> > >
> > > > + /* Only one open per device at a time */
> > > > + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> > > > + atomic_inc(&xsdfec->open_count);
> > > > + return -EBUSY;
> > > > + }
> > >
> > > What is that limitation for? Is it worse to open it twice than
> > > to dup() or fork()?
> > >
> > The device can be opened only once.
>
> What I mean here is that preventing the double open() is
> a fairly weak protection: it means you cannot have multiple
> 'struct file' pointers attached to the same inode, but you
> can still have the same 'struct file' being available to
> multiple processes.
>
Could you please suggest the solution?
My intention was to prevent more than one process access the same device.

> Arnd


Thanks
Dragan

2019-03-19 19:47:26

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl

On Tue, Mar 19, 2019 at 7:10 PM Dragan Cvetic <[email protected]> wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:[email protected]]
> > Sent: Tuesday 19 March 2019 15:36
> > To: Dragan Cvetic <[email protected]>
> > Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Linux ARM <[email protected]>;
> > Derek Kiernan <[email protected]>; Linux Kernel Mailing List <[email protected]>
> > Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl
> >
> > On Tue, Mar 19, 2019 at 3:59 PM Dragan Cvetic <[email protected]> wrote:
> > > >
> > > > > + /* Only one open per device at a time */
> > > > > + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> > > > > + atomic_inc(&xsdfec->open_count);
> > > > > + return -EBUSY;
> > > > > + }
> > > >
> > > > What is that limitation for? Is it worse to open it twice than
> > > > to dup() or fork()?
> > > >
> > > The device can be opened only once.
> >
> > What I mean here is that preventing the double open() is
> > a fairly weak protection: it means you cannot have multiple
> > 'struct file' pointers attached to the same inode, but you
> > can still have the same 'struct file' being available to
> > multiple processes.
> >
> Could you please suggest the solution?
> My intention was to prevent more than one process access the same device.

Generally speaking, you can't prevent it, but you should make sure that
if two processes attempt to use the same device, nothing bad happens.
Usually it's enough to have appropriate locking.

Arnd

2019-04-09 08:36:33

by Dragan Cvetic

[permalink] [raw]
Subject: RE: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl



> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Tuesday 19 March 2019 19:46
> To: Dragan Cvetic <[email protected]>
> Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Derek Kiernan <[email protected]>; Linux ARM
> <[email protected]>; Linux Kernel Mailing List <[email protected]>
> Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl
>
> On Tue, Mar 19, 2019 at 7:10 PM Dragan Cvetic <[email protected]> wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:[email protected]]
> > > Sent: Tuesday 19 March 2019 15:36
> > > To: Dragan Cvetic <[email protected]>
> > > Cc: gregkh <[email protected]>; Michal Simek <[email protected]>; Linux ARM <linux-arm-
> [email protected]>;
> > > Derek Kiernan <[email protected]>; Linux Kernel Mailing List <[email protected]>
> > > Subject: Re: [PATCH 04/12] misc: xilinx_sdfec: Add open, close and ioctl
> > >
> > > On Tue, Mar 19, 2019 at 3:59 PM Dragan Cvetic <[email protected]> wrote:
> > > > >
> > > > > > + /* Only one open per device at a time */
> > > > > > + if (!atomic_dec_and_test(&xsdfec->open_count)) {
> > > > > > + atomic_inc(&xsdfec->open_count);
> > > > > > + return -EBUSY;
> > > > > > + }
> > > > >
> > > > > What is that limitation for? Is it worse to open it twice than
> > > > > to dup() or fork()?
> > > > >
> > > > The device can be opened only once.
> > >
> > > What I mean here is that preventing the double open() is
> > > a fairly weak protection: it means you cannot have multiple
> > > 'struct file' pointers attached to the same inode, but you
> > > can still have the same 'struct file' being available to
> > > multiple processes.
> > >
> > Could you please suggest the solution?
> > My intention was to prevent more than one process access the same device.
>
> Generally speaking, you can't prevent it, but you should make sure that
> if two processes attempt to use the same device, nothing bad happens.
> Usually it's enough to have appropriate locking.
>
There is a need to increase the driver security, even the proposed is not perfect,
it is acceptable for us for now.

> Arnd