* [PATCH v5 0/2]Adds status interface for zynqmp-fpga @ 2023-01-31 9:48 Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 1/2] firmware: xilinx: Add pm api function for PL config reg readback Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface Nava kishore Manne 0 siblings, 2 replies; 7+ messages in thread From: Nava kishore Manne @ 2023-01-31 9:48 UTC (permalink / raw) To: michal.simek, mdf, hao.wu, yilun.xu, trix, nava.kishore.manne, ronak.jain, gregkh, tanmay.shah, ben.levinsky, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga Adds status interface for zynqmp-fpga, It's a read only interface which allows the user to get the Programmable Logic(PL) status. -Device Initialization error. -Device internal signal error. -All I/Os are placed in High-Z state. -Device start-up sequence error. -Firmware error. For more details refer the ug570. https://docs.xilinx.com/v/u/en-US/ug570-ultrascale-configuration Nava kishore Manne (2): firmware: xilinx: Add pm api function for PL config reg readback fpga: zynqmp-fpga: Adds status interface .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ drivers/firmware/xilinx/zynqmp.c | 33 +++++++ drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 11 +++ 4 files changed, 149 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] firmware: xilinx: Add pm api function for PL config reg readback 2023-01-31 9:48 [PATCH v5 0/2]Adds status interface for zynqmp-fpga Nava kishore Manne @ 2023-01-31 9:48 ` Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface Nava kishore Manne 1 sibling, 0 replies; 7+ messages in thread From: Nava kishore Manne @ 2023-01-31 9:48 UTC (permalink / raw) To: michal.simek, mdf, hao.wu, yilun.xu, trix, nava.kishore.manne, ronak.jain, gregkh, tanmay.shah, ben.levinsky, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga Adds PM API for performing Programmable Logic(PL) configuration register readback. It provides an interface to the firmware(pmufw) to readback the FPGA configuration register. Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com> --- changes for v2: - None. Changes for v3: - Updated API and config reg read-back handling logic - Updated the commit msg to align with the changes. Changes for v4: - Fixed some minor coding issues. No functional changes. - Updated Return value comments as suggested by Xu Yilun. Changes for v5: - Fixed some minor coding issues as suggested by Xu Yilun. No functional changes. drivers/firmware/xilinx/zynqmp.c | 33 ++++++++++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 11 ++++++++++ 2 files changed, 44 insertions(+) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index 129f68d7a6f5..3d8cc6795b43 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -948,6 +948,39 @@ int zynqmp_pm_fpga_get_status(u32 *value) } EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status); +/** + * zynqmp_pm_fpga_get_config_status - Get the FPGA configuration status. + * @value: Buffer to store FPGA configuration status. + * + * This function provides access to the pmufw to get the FPGA configuration + * status + * + * Return: 0 on success, a negative value on error + */ +int zynqmp_pm_fpga_get_config_status(u32 *value) +{ + u32 ret_payload[PAYLOAD_ARG_CNT]; + u32 buf, lower_addr, upper_addr; + int ret; + + if (!value) + return -EINVAL; + + lower_addr = lower_32_bits((u64)&buf); + upper_addr = upper_32_bits((u64)&buf); + + ret = zynqmp_pm_invoke_fn(PM_FPGA_READ, + XILINX_ZYNQMP_PM_FPGA_CONFIG_STAT_OFFSET, + lower_addr, upper_addr, + XILINX_ZYNQMP_PM_FPGA_READ_CONFIG_REG, + ret_payload); + + *value = ret_payload[1]; + + return ret; +} +EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_config_status); + /** * zynqmp_pm_pinctrl_request - Request Pin from firmware * @pin: Pin number to request diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h index b09f443d3ab9..ce37d55ffa44 100644 --- a/include/linux/firmware/xlnx-zynqmp.h +++ b/include/linux/firmware/xlnx-zynqmp.h @@ -71,6 +71,10 @@ #define XILINX_ZYNQMP_PM_FPGA_FULL 0x0U #define XILINX_ZYNQMP_PM_FPGA_PARTIAL BIT(0) +/* FPGA Status Reg */ +#define XILINX_ZYNQMP_PM_FPGA_CONFIG_STAT_OFFSET 7U +#define XILINX_ZYNQMP_PM_FPGA_READ_CONFIG_REG 0U + /* * Node IDs for the Error Events. */ @@ -120,6 +124,7 @@ enum pm_api_id { PM_CLOCK_GETRATE = 42, PM_CLOCK_SETPARENT = 43, PM_CLOCK_GETPARENT = 44, + PM_FPGA_READ = 46, PM_SECURE_AES = 47, PM_FEATURE_CHECK = 63, }; @@ -515,6 +520,7 @@ int zynqmp_pm_aes_engine(const u64 address, u32 *out); int zynqmp_pm_sha_hash(const u64 address, const u32 size, const u32 flags); int zynqmp_pm_fpga_load(const u64 address, const u32 size, const u32 flags); int zynqmp_pm_fpga_get_status(u32 *value); +int zynqmp_pm_fpga_get_config_status(u32 *value); int zynqmp_pm_write_ggs(u32 index, u32 value); int zynqmp_pm_read_ggs(u32 index, u32 *value); int zynqmp_pm_write_pggs(u32 index, u32 value); @@ -721,6 +727,11 @@ static inline int zynqmp_pm_fpga_get_status(u32 *value) return -ENODEV; } +static inline int zynqmp_pm_fpga_get_config_status(u32 *value) +{ + return -ENODEV; +} + static inline int zynqmp_pm_write_ggs(u32 index, u32 value) { return -ENODEV; -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface 2023-01-31 9:48 [PATCH v5 0/2]Adds status interface for zynqmp-fpga Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 1/2] firmware: xilinx: Add pm api function for PL config reg readback Nava kishore Manne @ 2023-01-31 9:48 ` Nava kishore Manne 2023-01-31 11:42 ` Greg KH 2023-02-10 9:18 ` Xu Yilun 1 sibling, 2 replies; 7+ messages in thread From: Nava kishore Manne @ 2023-01-31 9:48 UTC (permalink / raw) To: michal.simek, mdf, hao.wu, yilun.xu, trix, nava.kishore.manne, ronak.jain, gregkh, tanmay.shah, ben.levinsky, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga 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. Changes for v4: - Limit the error strings to one word for each as suggested by Xu Yilun Changes for v5: - Added new sysfs-driver-zynqmp-fpga file. .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga diff --git a/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga new file mode 100644 index 000000000000..66c98bb8dfba --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga @@ -0,0 +1,18 @@ +What: /sys/class/fpga_manager/<fpga>/device/status +Date: Jan 2023 +KernelVersion: 6.2 +Contact: Nava kishore Manne <nava.kishore.manne@amd.com> +Description: Read fpga status as a string. + If FPGA programming operation fails, it could be caused by crc + error or incompatible bitstream image. The intent of this + interface is to provide more detailed information for FPGA + programming errors to userspace. This is a list of strings for + the supported status. + + * CRC-Error - CRC error detected by hardware. + * Security-Error - Security error detected by hardware. + * Initialization-Error - Device Initialization error. + * Internal-Signal-Error - Device internal signal error. + * I/Os-High-Z-state - All I/Os are placed in High-Z state. + * Sequence-Error - Device start-up sequence error. + * Firmware-Error - Firmware error. diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c index c60f20949c47..81d3e18527ee 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] = "CRC-Error", + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "Security-Error", + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Initialization-Error", + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Internal-Signal-Error", + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "I/Os-High-Z-state", + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Sequence-Error", + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "Firmware-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); -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface 2023-01-31 9:48 ` [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface Nava kishore Manne @ 2023-01-31 11:42 ` Greg KH 2023-02-07 9:23 ` Manne, Nava kishore 2023-02-10 9:18 ` Xu Yilun 1 sibling, 1 reply; 7+ messages in thread From: Greg KH @ 2023-01-31 11:42 UTC (permalink / raw) To: Nava kishore Manne Cc: michal.simek, mdf, hao.wu, yilun.xu, trix, ronak.jain, tanmay.shah, ben.levinsky, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga On Tue, Jan 31, 2023 at 03:18:10PM +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. > > Changes for v4: > - Limit the error strings to one word for each as suggested by > Xu Yilun > > Changes for v5: > - Added new sysfs-driver-zynqmp-fpga file. > > .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ > drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ > 2 files changed, 105 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > diff --git a/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > new file mode 100644 > index 000000000000..66c98bb8dfba > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > @@ -0,0 +1,18 @@ > +What: /sys/class/fpga_manager/<fpga>/device/status > +Date: Jan 2023 > +KernelVersion: 6.2 > +Contact: Nava kishore Manne <nava.kishore.manne@amd.com> > +Description: Read fpga status as a string. > + If FPGA programming operation fails, it could be caused by crc > + error or incompatible bitstream image. The intent of this > + interface is to provide more detailed information for FPGA > + programming errors to userspace. This is a list of strings for > + the supported status. > + > + * CRC-Error - CRC error detected by hardware. > + * Security-Error - Security error detected by hardware. > + * Initialization-Error - Device Initialization error. > + * Internal-Signal-Error - Device internal signal error. > + * I/Os-High-Z-state - All I/Os are placed in High-Z state. > + * Sequence-Error - Device start-up sequence error. > + * Firmware-Error - Firmware error. > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > index c60f20949c47..81d3e18527ee 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] = "CRC-Error", > + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "Security-Error", > + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Initialization-Error", > + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Internal-Signal-Error", > + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "I/Os-High-Z-state", > + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Sequence-Error", > + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "Firmware-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]); > + Sorry, but no, you can not export a list of strings that userspace is somehow going to parse. As this is all a bit field, why not just export the raw hex value instead and let userspace parse what bits are set? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface 2023-01-31 11:42 ` Greg KH @ 2023-02-07 9:23 ` Manne, Nava kishore 2023-02-07 16:34 ` Xu Yilun 0 siblings, 1 reply; 7+ messages in thread From: Manne, Nava kishore @ 2023-02-07 9:23 UTC (permalink / raw) To: Greg KH, yilun.xu Cc: michal.simek, mdf, hao.wu, trix, ronak.jain, tanmay.shah, Levinsky, Ben, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga Hi Greg, Thanks for providing the review comments. Please find my response inline. > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, January 31, 2023 5:13 PM > To: Manne, Nava kishore <nava.kishore.manne@amd.com> > Cc: michal.simek@xilinx.com; mdf@kernel.org; hao.wu@intel.com; > yilun.xu@intel.com; trix@redhat.com; ronak.jain@xilinx.com; > tanmay.shah@xilinx.com; Levinsky, Ben <ben.levinsky@amd.com>; > rajan.vaja@xilinx.com; harsha.harsha@xilinx.com; > mathieu.poirier@linaro.org; lakshmi.sai.krishna.potthuri@xilinx.com; > arnd@arndb.de; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; linux-fpga@vger.kernel.org > Subject: Re: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface > > On Tue, Jan 31, 2023 at 03:18:10PM +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. > > > > Changes for v4: > > - Limit the error strings to one word for each as suggested by > > Xu Yilun > > > > Changes for v5: > > - Added new sysfs-driver-zynqmp-fpga file. > > > > .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ > > drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ > > 2 files changed, 105 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > new file mode 100644 > > index 000000000000..66c98bb8dfba > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > @@ -0,0 +1,18 @@ > > +What: /sys/class/fpga_manager/<fpga>/device/status > > +Date: Jan 2023 > > +KernelVersion: 6.2 > > +Contact: Nava kishore Manne <nava.kishore.manne@amd.com> > > +Description: Read fpga status as a string. > > + If FPGA programming operation fails, it could be caused by > crc > > + error or incompatible bitstream image. The intent of this > > + interface is to provide more detailed information for FPGA > > + programming errors to userspace. This is a list of strings for > > + the supported status. > > + > > + * CRC-Error - CRC error detected by hardware. > > + * Security-Error - Security error detected by hardware. > > + * Initialization-Error - Device Initialization error. > > + * Internal-Signal-Error - Device internal signal error. > > + * I/Os-High-Z-state - All I/Os are placed in High-Z state. > > + * Sequence-Error - Device start-up sequence error. > > + * Firmware-Error - Firmware error. > > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > > index c60f20949c47..81d3e18527ee 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] = "CRC-Error", > > + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "Security-Error", > > + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Initialization-Error", > > + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Internal-Signal-Error", > > + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "I/Os-High-Z-state", > > + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Sequence-Error", > > + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "Firmware- > 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]); > > + > > Sorry, but no, you can not export a list of strings that userspace is somehow > going to parse. > > As this is all a bit field, why not just export the raw hex value instead and let > userspace parse what bits are set? > This implementation is align with the fpga core status interface(https://git.kernel.org/pub/scm/linux/kernel/git/fpga/linux-fpga.git/tree/drivers/fpga/fpga-mgr.c?h=for-6.2#n632) I Agree to export the raw hex value instead of multiple error strings. In case of exporting the raw hex value for status can we update this fpga-mgr.c core file (status_show() API) instead of creating individual sysfs file per driver/device? and with this proposed implementation will also fix the issues/Limitations with generic status interface. Existing limitation with the generic fpga status interface: Different vendors have different error sets defined by Hardware. If we always define the new bits when we cannot find an exact 1:1 mapping in the core the 64 bits would soon be used out. Xu Yilun: Can you please share your thoughts here? Regards, Navakishore. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface 2023-02-07 9:23 ` Manne, Nava kishore @ 2023-02-07 16:34 ` Xu Yilun 0 siblings, 0 replies; 7+ messages in thread From: Xu Yilun @ 2023-02-07 16:34 UTC (permalink / raw) To: Manne, Nava kishore Cc: Greg KH, michal.simek, mdf, hao.wu, trix, ronak.jain, tanmay.shah, Levinsky, Ben, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga On 2023-02-07 at 09:23:12 +0000, Manne, Nava kishore wrote: > Hi Greg, > > Thanks for providing the review comments. > Please find my response inline. > > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Tuesday, January 31, 2023 5:13 PM > > To: Manne, Nava kishore <nava.kishore.manne@amd.com> > > Cc: michal.simek@xilinx.com; mdf@kernel.org; hao.wu@intel.com; > > yilun.xu@intel.com; trix@redhat.com; ronak.jain@xilinx.com; > > tanmay.shah@xilinx.com; Levinsky, Ben <ben.levinsky@amd.com>; > > rajan.vaja@xilinx.com; harsha.harsha@xilinx.com; > > mathieu.poirier@linaro.org; lakshmi.sai.krishna.potthuri@xilinx.com; > > arnd@arndb.de; linux-arm-kernel@lists.infradead.org; linux- > > kernel@vger.kernel.org; linux-fpga@vger.kernel.org > > Subject: Re: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface > > > > On Tue, Jan 31, 2023 at 03:18:10PM +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. > > > > > > Changes for v4: > > > - Limit the error strings to one word for each as suggested by > > > Xu Yilun > > > > > > Changes for v5: > > > - Added new sysfs-driver-zynqmp-fpga file. > > > > > > .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ > > > drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ > > > 2 files changed, 105 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > > b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > > new file mode 100644 > > > index 000000000000..66c98bb8dfba > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > > @@ -0,0 +1,18 @@ > > > +What: /sys/class/fpga_manager/<fpga>/device/status > > > +Date: Jan 2023 > > > +KernelVersion: 6.2 > > > +Contact: Nava kishore Manne <nava.kishore.manne@amd.com> > > > +Description: Read fpga status as a string. > > > + If FPGA programming operation fails, it could be caused by > > crc > > > + error or incompatible bitstream image. The intent of this > > > + interface is to provide more detailed information for FPGA > > > + programming errors to userspace. This is a list of strings for > > > + the supported status. > > > + > > > + * CRC-Error - CRC error detected by hardware. > > > + * Security-Error - Security error detected by hardware. > > > + * Initialization-Error - Device Initialization error. > > > + * Internal-Signal-Error - Device internal signal error. > > > + * I/Os-High-Z-state - All I/Os are placed in High-Z state. > > > + * Sequence-Error - Device start-up sequence error. > > > + * Firmware-Error - Firmware error. > > > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > > > index c60f20949c47..81d3e18527ee 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] = "CRC-Error", > > > + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "Security-Error", > > > + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Initialization-Error", > > > + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Internal-Signal-Error", > > > + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "I/Os-High-Z-state", > > > + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Sequence-Error", > > > + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "Firmware- > > 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]); > > > + > > > > Sorry, but no, you can not export a list of strings that userspace is somehow > > going to parse. > > > > As this is all a bit field, why not just export the raw hex value instead and let > > userspace parse what bits are set? > > > > This implementation is align with the fpga core status interface(https://git.kernel.org/pub/scm/linux/kernel/git/fpga/linux-fpga.git/tree/drivers/fpga/fpga-mgr.c?h=for-6.2#n632) > I Agree to export the raw hex value instead of multiple error strings. I also agree, it is a much simpler implementation. > In case of exporting the raw hex value for status can we update this fpga-mgr.c core file (status_show() API) > instead of creating individual sysfs file per driver/device? and with this proposed implementation But I don't think we should change format of the existing 'status' sysfs node, it impacts the existing users. Just freeze it and add no new user. > will also fix the issues/Limitations with generic status interface. > > Existing limitation with the generic fpga status interface: > Different vendors have different error sets defined by Hardware. If we always define the new bits when we cannot find an exact 1:1 > mapping in the core the 64 bits would soon be used out. Even if we change the status interface to raw hex output, issues are still there. I guess you may want to output bits specifically defined by each driver, but then it makes no sense to have this generic class interface. Users can't tell what the status is by just reading the interface. So as I mentioned before, my preference is driver creates its own sysfs interface for its own hardware status if needed. Thanks, Yilun > > Xu Yilun: Can you please share your thoughts here? > > Regards, > Navakishore. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface 2023-01-31 9:48 ` [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface Nava kishore Manne 2023-01-31 11:42 ` Greg KH @ 2023-02-10 9:18 ` Xu Yilun 1 sibling, 0 replies; 7+ messages in thread From: Xu Yilun @ 2023-02-10 9:18 UTC (permalink / raw) To: Nava kishore Manne Cc: michal.simek, mdf, hao.wu, trix, ronak.jain, gregkh, tanmay.shah, ben.levinsky, rajan.vaja, harsha.harsha, mathieu.poirier, lakshmi.sai.krishna.potthuri, arnd, linux-arm-kernel, linux-kernel, linux-fpga On 2023-01-31 at 15:18:10 +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 Please don't use the 'class' path, this is not a class interface. And people still have no idea how to reach the zynqmp-fpga status interface with this example path. I suggest: cat /sys/bus/platform/drivers/... > > 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. > > Changes for v4: > - Limit the error strings to one word for each as suggested by > Xu Yilun > > Changes for v5: > - Added new sysfs-driver-zynqmp-fpga file. > > .../ABI/testing/sysfs-driver-zynqmp-fpga | 18 ++++ > drivers/fpga/zynqmp-fpga.c | 87 +++++++++++++++++++ > 2 files changed, 105 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > > diff --git a/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > new file mode 100644 > index 000000000000..66c98bb8dfba > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-zynqmp-fpga > @@ -0,0 +1,18 @@ > +What: /sys/class/fpga_manager/<fpga>/device/status Same concern > +Date: Jan 2023 > +KernelVersion: 6.2 > +Contact: Nava kishore Manne <nava.kishore.manne@amd.com> > +Description: Read fpga status as a string. > + If FPGA programming operation fails, it could be caused by crc > + error or incompatible bitstream image. The intent of this > + interface is to provide more detailed information for FPGA > + programming errors to userspace. This is a list of strings for > + the supported status. > + > + * CRC-Error - CRC error detected by hardware. > + * Security-Error - Security error detected by hardware. > + * Initialization-Error - Device Initialization error. > + * Internal-Signal-Error - Device internal signal error. > + * I/Os-High-Z-state - All I/Os are placed in High-Z state. > + * Sequence-Error - Device start-up sequence error. > + * Firmware-Error - Firmware error. > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > index c60f20949c47..81d3e18527ee 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] = "CRC-Error", > + [ZYNQMP_FPGA_STATUS_SECURITY_ERR] = "Security-Error", > + [ZYNQMP_FPGA_STATUS_DEVICE_INIT_ERR] = "Initialization-Error", > + [ZYNQMP_FPGA_STATUS_SIGNAL_ERR] = "Internal-Signal-Error", > + [ZYNQMP_FPGA_STATUS_HIGH_Z_STATE_ERR] = "I/Os-High-Z-state", > + [ZYNQMP_FPGA_STATUS_EOS_ERR] = "Sequence-Error", > + [ZYNQMP_FPGA_MGR_STATUS_FIRMWARE_REQ_ERR] = "Firmware-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); Just curious if there will be more than one zynq fpga devices in a system? if yes, how to distinguish their status? > + 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); Again, please do fix this as mentioned by Greg: https://lore.kernel.org/all/Y0fYjyXrMEo6M76k@kroah.com/ Check how other platform drivers are doing. And this may also be useful: http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/ Thanks, Yilun > + 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-10 9:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-31 9:48 [PATCH v5 0/2]Adds status interface for zynqmp-fpga Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 1/2] firmware: xilinx: Add pm api function for PL config reg readback Nava kishore Manne 2023-01-31 9:48 ` [PATCH v5 2/2] fpga: zynqmp-fpga: Adds status interface Nava kishore Manne 2023-01-31 11:42 ` Greg KH 2023-02-07 9:23 ` Manne, Nava kishore 2023-02-07 16:34 ` Xu Yilun 2023-02-10 9:18 ` Xu Yilun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).