| Message ID | 20221013090556.741357-3-nava.kishore.manne@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Adds status interface for zynqmp-fpga | expand |
On Thu, Oct 13, 2022 at 02:35:56PM +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@amd.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(+) You add sysfs files without a Documentation/ABI/ update as well, which is not allowed. Please fix that up for your next submission. > @@ -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; > + } You just raced with userspace and lost. Do not do this, set the default groups attribute in your platform driver and the driver core will handle this all for you automatically. thanks, greg k-h
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@amd.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 >
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", +}; + /** * 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);
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@amd.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(+)