Re: [PATCH 07/19] bus/cdx: add device attributes
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2023-01-17 14:14:11
Also in:
linux-devicetree, linux-kbuild, lkml
On Tue, Jan 17, 2023 at 07:11:39PM +0530, Nipun Gupta wrote:
quoted hunk ↗ jump to hunk
Create sysfs entry for CDX devices. Sysfs entries provided in each of the CDX device detected by the CDX controller - vendor id - device id - remove - reset of the device. - driver override Signed-off-by: Puneet Gupta <redacted> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> Signed-off-by: Tarak Reddy <redacted> --- Documentation/ABI/testing/sysfs-bus-cdx | 34 +++++ drivers/bus/cdx/cdx.c | 144 ++++++++++++++++++++ drivers/bus/cdx/controller/cdx_controller.c | 19 +++ drivers/bus/cdx/controller/mcdi_functions.c | 14 ++ drivers/bus/cdx/controller/mcdi_functions.h | 11 ++ include/linux/cdx/cdx_bus.h | 23 ++++ 6 files changed, 245 insertions(+)diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx index 8c2425fdb6d9..1e0fdce18cde 100644 --- a/Documentation/ABI/testing/sysfs-bus-cdx +++ b/Documentation/ABI/testing/sysfs-bus-cdx@@ -10,3 +10,37 @@ Description: For example:: # echo 1 > /sys/bus/cdx/rescan + +What: /sys/bus/cdx/devices/.../vendor +Date: January 2023 +Contact: nipun.gupta@amd.com +Description: + Vendor ID for this CDX device
What format is this in? How big is it? Examples?
+ +What: /sys/bus/cdx/devices/.../device +Date: January 2023 +Contact: nipun.gupta@amd.com +Description: + Device ID for this CDX device
Same here, be specific.
+ +What: /sys/bus/cdx/devices/.../reset +Date: January 2023 +Contact: nipun.gupta@amd.com +Description: + Writing a non-zero value to this file would reset the + CDX device + + For example:: + + # echo 1 > /sys/bus/cdx/.../reset
Will that remove the device from the driver too? Again, more documentation please.
+ +What: /sys/bus/cdx/devices/.../remove +Date: January 2023 +Contact: tarak.reddy@amd.com +Description: + Writing a non-zero value to this file would remove the + corrosponding device from the CDX bus + + For example:: + + # echo 1 > /sys/bus/cdx/devices/.../remove
Why would you want to remove a device from the bus like this?
quoted hunk ↗ jump to hunk
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c index 2737470f08d3..1372d8dcaa4c 100644 --- a/drivers/bus/cdx/cdx.c +++ b/drivers/bus/cdx/cdx.c@@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map, MAX_CDX_CONTROLLERS); /* List of CDX controllers registered with the CDX bus */ static LIST_HEAD(cdx_controllers); +/** + * reset_cdx_device - Reset a CDX device + * @dev: CDX device + * @data: This is always passed as NULL, and is not used in this API, + * but is required here as the bus_for_each_dev() API expects + * the passed function (reset_cdx_device) to have this + * as an argument. + * + * @return: -errno on failure, 0 on success.
I recommend this function actually be the one without the data pointer, that way you don't get confused here.
+ */
+static int reset_cdx_device(struct device *dev, void *data)
+{
+ struct cdx_device *cdx_dev = to_cdx_device(dev);
+ struct cdx_controller *cdx = cdx_dev->cdx;
+ struct cdx_device_config dev_config;
+ int ret;
+
+ dev_config.type = CDX_DEV_RESET_CONF;
+ ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
+ cdx_dev->dev_num, &dev_config);
+ if (ret)
+ dev_err(dev, "cdx device reset failed\n");
+
+ return ret;
+}
+
+int cdx_dev_reset(struct device *dev)
+{
+ return reset_cdx_device(dev, NULL);
+}
+EXPORT_SYMBOL_GPL(cdx_dev_reset);Remove the indirection as mentioned above please. Otherwise, very minor comments, nice work. greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel