[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y0mILkB9ym5fQ4wJ@yilunxu-OptiPlex-7050>
Date: Sat, 15 Oct 2022 00:02:54 +0800
From: Xu Yilun <yilun.xu@...el.com>
To: Nava kishore Manne <nava.kishore.manne@....com>
Cc: git@...inx.com, michal.simek@...inx.com, mdf@...nel.org,
hao.wu@...el.com, trix@...hat.com, gregkh@...uxfoundation.org,
ronak.jain@...inx.com, rajan.vaja@...inx.com,
abhyuday.godhasara@...inx.com, harsha.harsha@...inx.com,
lakshmi.sai.krishna.potthuri@...inx.com, tanmay.shah@...inx.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-fpga@...r.kernel.org
Subject: Re: [PATCH v3 2/2] fpga: zynqmp-fpga: Adds status interface
On 2022-10-13 at 14:35:56 +0530, Nava kishore Manne wrote:
> Adds status interface for zynqmp-fpga, It's a read only interface
> which allows the user to get the Programmable Logic(PL) configuration
> status.
>
> Usage:
> To read the Programmable Logic(PL) configuration status
> cat /sys/class/fpga_manager/<fpga>/device/status
>
> Signed-off-by: Nava kishore Manne <nava.kishore.manne@....com>
> ---
> Changes for v2:
> - Updated status messages handling logic as suggested by Xu Yilun.
>
> Changes for v3:
> - Updated status interface handling logic (Restrict the status
> interface to the device-specific instead of handled by the core)
> as suggested by Xu Yilun.
>
> drivers/fpga/zynqmp-fpga.c | 87 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> index c60f20949c47..4e0295486c36 100644
> --- a/drivers/fpga/zynqmp-fpga.c
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -15,6 +15,37 @@
> /* Constant Definitions */
> #define IXR_FPGA_DONE_MASK BIT(3)
>
> +/* Error Register */
> +#define IXR_FPGA_ERR_CRC_ERR BIT(0)
> +#define IXR_FPGA_ERR_SECURITY_ERR BIT(16)
> +
> +/* Signal Status Register */
> +#define IXR_FPGA_END_OF_STARTUP BIT(4)
> +#define IXR_FPGA_GST_CFG_B BIT(5)
> +#define IXR_FPGA_INIT_B_INTERNAL BIT(11)
> +#define IXR_FPGA_DONE_INTERNAL_SIGNAL BIT(13)
> +
> +/* FPGA error status. */
> +enum {
> + ZYNQMP_FPGA_STATUS_CRC_ERR,
> + ZYNQMP_FPGA_STATUS_SECURITY_ERR,
> + ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR,
> + ZYNQMP_FPGA_STATUS_SIGNAL_ERR,
> + ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR,
> + ZYNQMP_FPGA_STATUS_EOS_ERR,
> + ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR,
> +};
> +
> +static const char * const zynqmp_fpga_error_statuses[] = {
> + [ZYNQMP_FPGA_STATUS_CRC_ERR] = "reconfig CRC error",
> + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "reconfig security error",
> + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Device Initialization error",
> + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Device internal signal error",
> + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "All I/Os are placed in High-Z state",
> + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Device sequence error",
> + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "firmware request error",
If you have multiple errors, the output would be a mess. You can not
even tell how many errors are there.
For string values, I suggest one word for each, and separated by a space.
Thanks,
Yilun
> +};
> +
> /**
> * struct zynqmp_fpga_priv - Private data structure
> * @dev: Device data structure
> @@ -77,6 +108,54 @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
> return FPGA_MGR_STATE_UNKNOWN;
> }
>
> +static ssize_t status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + unsigned long status = 0;
> + ssize_t len = 0;
> + u32 reg_val;
> + int ret;
> + u8 i;
> +
> + ret = zynqmp_pm_fpga_get_config_status(®_val);
> + if (!ret) {
> + if (reg_val & IXR_FPGA_ERR_CRC_ERR)
> + status |= ZYNQMP_FPGA_STATUS_CRC_ERR;
> + if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
> + status |= ZYNQMP_FPGA_STATUS_SECURITY_ERR;
> + if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
> + status |= ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR;
> + if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
> + status |= ZYNQMP_FPGA_STATUS_SIGNAL_ERR;
> + if (!(reg_val & IXR_FPGA_GST_CFG_B))
> + status |= ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR;
> + if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
> + status |= ZYNQMP_FPGA_STATUS_EOS_ERR;
> + } else {
> + status = ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
> + }
> +
> + for_each_set_bit(i, &status, ARRAY_SIZE(zynqmp_fpga_error_statuses))
> + len += sysfs_emit_at(buf, len, "%s ",
> + zynqmp_fpga_error_statuses[i]);
> +
> + if (len)
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +
> +static DEVICE_ATTR_RO(status);
> +
> +static struct attribute *zynqmp_fpga_device_attrs[] = {
> + &dev_attr_status.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group zynqmp_fpga_attr_group = {
> + .attrs = zynqmp_fpga_device_attrs,
> +};
> +
> static const struct fpga_manager_ops zynqmp_fpga_ops = {
> .state = zynqmp_fpga_ops_state,
> .write_init = zynqmp_fpga_ops_write_init,
> @@ -88,6 +167,7 @@ static int zynqmp_fpga_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct zynqmp_fpga_priv *priv;
> struct fpga_manager *mgr;
> + int ret;
>
> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -95,6 +175,13 @@ static int zynqmp_fpga_probe(struct platform_device *pdev)
>
> priv->dev = dev;
>
> + /* Add the device attributes */
> + ret = sysfs_create_group(&dev->kobj, &zynqmp_fpga_attr_group);
> + if (ret) {
> + dev_err(dev, "Error creating sysfs files\n");
> + return ret;
> + }
> +
> mgr = devm_fpga_mgr_register(dev, "Xilinx ZynqMP FPGA Manager",
> &zynqmp_fpga_ops, priv);
> return PTR_ERR_OR_ZERO(mgr);
> --
> 2.25.1
>
Powered by blists - more mailing lists