2021-12-03 09:18:44

by Ronak Jain

[permalink] [raw]
Subject: [PATCH v3 0/3] Add support for runtime features

Add support for runtime feature configuration by using the IOCTL
calls. The user can enable or disable as well as can configure the
runtime features. The support is added for the over temperature and
external watchdog. The user can configure the over temperature limit
and external watchdog timer interval at runtime by using PM_IOCTL
calls.

Also, added support for sysfs interface for runtime feature
configuration for the over temperature and external watchdog features.

Added zynqmp_devinfo structure to handle muliple driver instances
when accessed by multiple devices.

Changes in v3:
- Added zynqmp_devinfo structure to store device instances
- Modified feature_conf_id from atomic variable to u32
- Update commit message
- Resolved merge conflict

Changes in v2:
- Remove default footer from email
- Update commit message
- Resolved merged conflict

Ronak Jain (3):
firmware: xilinx: Add support for runtime features
firmware: zynqmp: Add sysfs entry for runtime features
firmware: xilinx: Add sysfs support for feature config

.../ABI/stable/sysfs-driver-firmware-zynqmp | 84 ++++++++++++
drivers/firmware/xilinx/zynqmp.c | 120 ++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 25 ++++
3 files changed, 229 insertions(+)

--
2.32.0.93.g670b81a



2021-12-03 09:18:58

by Ronak Jain

[permalink] [raw]
Subject: [PATCH v3 1/3] firmware: xilinx: Add support for runtime features

Add support for runtime features by using an IOCTL call. The features
can be enabled or disabled on the firmware as well as the features
can be configured at runtime by querying IOCTL_SET_FEATURE_CONFIG id.
Similarly, the user can get the configured values of features by
querying IOCTL_GET_FEATURE_CONFIG id.

Here, the support is added for the over temperature and external
watchdog features only.

Signed-off-by: Ronak Jain <[email protected]>
---
Changes in v3:
- Resolved merged conflict

Changes in v2:
- Resolved merged conflict
- Update commit message
---
drivers/firmware/xilinx/zynqmp.c | 27 +++++++++++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 25 +++++++++++++++++++++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 3dd45a7420dc..6b05844e67d7 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -1129,6 +1129,33 @@ int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype)
0, 0, NULL);
}

+/**
+ * zynqmp_pm_set_feature_config - PM call to request IOCTL for feature config
+ * @id: The config ID of the feature to be configured
+ * @value: The config value of the feature to be configured
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_set_feature_config(enum pm_feature_config_id id, u32 value)
+{
+ return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_FEATURE_CONFIG,
+ id, value, NULL);
+}
+
+/**
+ * zynqmp_pm_get_feature_config - PM call to get value of configured feature
+ * @id: The config id of the feature to be queried
+ * @payload: Returned value array
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_get_feature_config(enum pm_feature_config_id id,
+ u32 *payload)
+{
+ return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_FEATURE_CONFIG,
+ id, 0, payload);
+}
+
/**
* struct zynqmp_pm_shutdown_scope - Struct for shutdown scope
* @subtype: Shutdown subtype
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 47fd4e52a423..772f7d6ba682 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -126,6 +126,9 @@ enum pm_ioctl_id {
/* Set healthy bit value */
IOCTL_SET_BOOT_HEALTH_STATUS = 17,
IOCTL_OSPI_MUX_SELECT = 21,
+ /* Runtime feature configuration */
+ IOCTL_SET_FEATURE_CONFIG = 26,
+ IOCTL_GET_FEATURE_CONFIG = 27,
};

enum pm_query_id {
@@ -359,6 +362,14 @@ enum ospi_mux_select_type {
PM_OSPI_MUX_SEL_LINEAR = 1,
};

+enum pm_feature_config_id {
+ PM_FEATURE_INVALID = 0,
+ PM_FEATURE_OVERTEMP_STATUS = 1,
+ PM_FEATURE_OVERTEMP_VALUE = 2,
+ PM_FEATURE_EXTWDT_STATUS = 3,
+ PM_FEATURE_EXTWDT_VALUE = 4,
+};
+
/**
* struct zynqmp_pm_query_data - PM query data
* @qid: query ID
@@ -427,6 +438,8 @@ int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
u32 value);
int zynqmp_pm_load_pdi(const u32 src, const u64 address);
+int zynqmp_pm_set_feature_config(enum pm_feature_config_id id, u32 value);
+int zynqmp_pm_get_feature_config(enum pm_feature_config_id id, u32 *payload);
#else
static inline int zynqmp_pm_get_api_version(u32 *version)
{
@@ -658,6 +671,18 @@ static inline int zynqmp_pm_load_pdi(const u32 src, const u64 address)
{
return -ENODEV;
}
+
+static inline int zynqmp_pm_set_feature_config(enum pm_feature_config_id id,
+ u32 value)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_get_feature_config(enum pm_feature_config_id id,
+ u32 *payload)
+{
+ return -ENODEV;
+}
#endif

#endif /* __FIRMWARE_ZYNQMP_H__ */
--
2.32.0.93.g670b81a


2021-12-03 09:19:09

by Ronak Jain

[permalink] [raw]
Subject: [PATCH v3 3/3] firmware: xilinx: Add sysfs support for feature config

Add support for sysfs interface for runtime features configuration.
The user can configure the features at runtime. First the user need
to select the config id of the supported features and then the user
can configure the parameters of the feature based on the config id.
So far the support is added for the over temperature and external
watchdog features.

Added zynqmp_devinfo structure to handle muliple driver instances
when accessed by multiple devices.

Signed-off-by: Ronak Jain <[email protected]>
---
Changes in v3:
- Added zynqmp_devinfo structure to store device instances
- Modified feature_conf_id from atomic variable to u32
- Update commit message
- Resolved merge conflicts

Changes in v2:
- Update commit message
---
drivers/firmware/xilinx/zynqmp.c | 93 ++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 6b05844e67d7..f37a8e805cd7 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -38,6 +38,16 @@
static bool feature_check_enabled;
static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);

+/**
+ * struct zynqmp_devinfo - Structure for Zynqmp device instance
+ * @dev: Device Pointer
+ * @feature_conf_id: Feature conf id
+ */
+struct zynqmp_devinfo {
+ struct device *dev;
+ u32 feature_conf_id;
+};
+
/**
* struct pm_api_feature_data - PM API Feature data
* @pm_api_id: PM API Id, used as key to index into hashmap
@@ -1424,6 +1434,78 @@ static DEVICE_ATTR_RW(pggs1);
static DEVICE_ATTR_RW(pggs2);
static DEVICE_ATTR_RW(pggs3);

+static ssize_t feature_config_id_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
+
+ return sysfs_emit(buf, "%d\n", devinfo->feature_conf_id);
+}
+
+static ssize_t feature_config_id_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u32 config_id;
+ int ret;
+ struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
+
+ if (!buf)
+ return -EINVAL;
+
+ ret = kstrtou32(buf, 10, &config_id);
+ if (ret)
+ return ret;
+
+ devinfo->feature_conf_id = config_id;
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(feature_config_id);
+
+static ssize_t feature_config_value_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ int ret;
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+ struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
+
+ ret = zynqmp_pm_get_feature_config(devinfo->feature_conf_id,
+ ret_payload);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%d\n", ret_payload[1]);
+}
+
+static ssize_t feature_config_value_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u32 value;
+ int ret;
+ struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
+
+ if (!buf)
+ return -EINVAL;
+
+ ret = kstrtou32(buf, 10, &value);
+ if (ret)
+ return ret;
+
+ ret = zynqmp_pm_set_feature_config(devinfo->feature_conf_id,
+ value);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(feature_config_value);
+
static struct attribute *zynqmp_firmware_attrs[] = {
&dev_attr_ggs0.attr,
&dev_attr_ggs1.attr,
@@ -1435,6 +1517,8 @@ static struct attribute *zynqmp_firmware_attrs[] = {
&dev_attr_pggs3.attr,
&dev_attr_shutdown_scope.attr,
&dev_attr_health_status.attr,
+ &dev_attr_feature_config_id.attr,
+ &dev_attr_feature_config_value.attr,
NULL,
};

@@ -1444,6 +1528,7 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *np;
+ struct zynqmp_devinfo *devinfo;
int ret;

np = of_find_compatible_node(NULL, NULL, "xlnx,zynqmp");
@@ -1460,6 +1545,14 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
if (ret)
return ret;

+ devinfo = devm_kzalloc(dev, sizeof(*devinfo), GFP_KERNEL);
+ if (!devinfo)
+ return -ENOMEM;
+
+ devinfo->dev = dev;
+
+ platform_set_drvdata(pdev, devinfo);
+
/* Check PM API version number */
zynqmp_pm_get_api_version(&pm_api_version);
if (pm_api_version < ZYNQMP_PM_VERSION) {
--
2.32.0.93.g670b81a


2021-12-03 09:22:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] firmware: xilinx: Add sysfs support for feature config

On Fri, Dec 03, 2021 at 01:18:14AM -0800, Ronak Jain wrote:
> Add support for sysfs interface for runtime features configuration.
> The user can configure the features at runtime. First the user need
> to select the config id of the supported features and then the user
> can configure the parameters of the feature based on the config id.
> So far the support is added for the over temperature and external
> watchdog features.
>
> Added zynqmp_devinfo structure to handle muliple driver instances
> when accessed by multiple devices.
>
> Signed-off-by: Ronak Jain <[email protected]>
> ---
> Changes in v3:
> - Added zynqmp_devinfo structure to store device instances
> - Modified feature_conf_id from atomic variable to u32
> - Update commit message
> - Resolved merge conflicts
>
> Changes in v2:
> - Update commit message
> ---
> drivers/firmware/xilinx/zynqmp.c | 93 ++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 6b05844e67d7..f37a8e805cd7 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -38,6 +38,16 @@
> static bool feature_check_enabled;
> static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
>
> +/**
> + * struct zynqmp_devinfo - Structure for Zynqmp device instance
> + * @dev: Device Pointer
> + * @feature_conf_id: Feature conf id
> + */
> +struct zynqmp_devinfo {
> + struct device *dev;
> + u32 feature_conf_id;
> +};
> +
> /**
> * struct pm_api_feature_data - PM API Feature data
> * @pm_api_id: PM API Id, used as key to index into hashmap
> @@ -1424,6 +1434,78 @@ static DEVICE_ATTR_RW(pggs1);
> static DEVICE_ATTR_RW(pggs2);
> static DEVICE_ATTR_RW(pggs3);
>
> +static ssize_t feature_config_id_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
> +
> + return sysfs_emit(buf, "%d\n", devinfo->feature_conf_id);
> +}

You are adding new sysfs files without any Documentation/ABI/ entries :(

Please fix.

thanks,

greg k-h

2021-12-03 10:16:36

by Ronak Jain

[permalink] [raw]
Subject: RE: [PATCH v3 3/3] firmware: xilinx: Add sysfs support for feature config

Hi Greg KH,

Thanks for reviewing the patch.

> -----Original Message-----
> From: Greg KH <[email protected]>
> Sent: Friday, December 3, 2021 2:52 PM
> To: Ronak Jain <[email protected]>
> Cc: Michal Simek <[email protected]>; [email protected]; Rajan
> Vaja <[email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected]; Sai Krishna Potthuri
> <[email protected]>
> Subject: Re: [PATCH v3 3/3] firmware: xilinx: Add sysfs support for feature
> config
>
> On Fri, Dec 03, 2021 at 01:18:14AM -0800, Ronak Jain wrote:
> > Add support for sysfs interface for runtime features configuration.
> > The user can configure the features at runtime. First the user need
> > to select the config id of the supported features and then the user
> > can configure the parameters of the feature based on the config id.
> > So far the support is added for the over temperature and external
> > watchdog features.
> >
> > Added zynqmp_devinfo structure to handle muliple driver instances
> > when accessed by multiple devices.
> >
> > Signed-off-by: Ronak Jain <[email protected]>
> > ---
> > Changes in v3:
> > - Added zynqmp_devinfo structure to store device instances
> > - Modified feature_conf_id from atomic variable to u32
> > - Update commit message
> > - Resolved merge conflicts
> >
> > Changes in v2:
> > - Update commit message
> > ---
> > drivers/firmware/xilinx/zynqmp.c | 93
> > ++++++++++++++++++++++++++++++++
> > 1 file changed, 93 insertions(+)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp.c
> > b/drivers/firmware/xilinx/zynqmp.c
> > index 6b05844e67d7..f37a8e805cd7 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -38,6 +38,16 @@
> > static bool feature_check_enabled;
> > static DEFINE_HASHTABLE(pm_api_features_map,
> > PM_API_FEATURE_CHECK_MAX_ORDER);
> >
> > +/**
> > + * struct zynqmp_devinfo - Structure for Zynqmp device instance
> > + * @dev: Device Pointer
> > + * @feature_conf_id: Feature conf id
> > + */
> > +struct zynqmp_devinfo {
> > + struct device *dev;
> > + u32 feature_conf_id;
> > +};
> > +
> > /**
> > * struct pm_api_feature_data - PM API Feature data
> > * @pm_api_id: PM API Id, used as key to index into hashmap
> > @@ -1424,6 +1434,78 @@ static DEVICE_ATTR_RW(pggs1); static
> > DEVICE_ATTR_RW(pggs2); static DEVICE_ATTR_RW(pggs3);
> >
> > +static ssize_t feature_config_id_show(struct device *device,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct zynqmp_devinfo *devinfo = dev_get_drvdata(device);
> > +
> > + return sysfs_emit(buf, "%d\n", devinfo->feature_conf_id); }
>
> You are adding new sysfs files without any Documentation/ABI/ entries :(
>
> Please fix.
As you pointed out the documentation is already there. However, it needs to be updated. I will update it.

Thanks,
Ronak
>
> thanks,
>
> greg k-h