misc: xilinx-ai-engine: split grouped sysfs init

The current implementation creates sysfs entries for both tiles and
partitions in the partition create. This patch separates the functionality
by creating sysfs entries when the tile and partition is initially created.

This patch also leverages the non-device managed api calls for creating the
sysfs group. This change is because we need fine control over the creation
and removal of the files from the system. We cannot leave it up to the
kernel since it will do it incorrectly.

Let Me Explain:

One issue that I'm seeing is when the device_del() is called. It will
execute the cleanup on a device which includes kobject_del(). This in turn
results in the kernel cleaning up the sysfs entries.

The issue is the sysfs group is managed by dev res. When the refcount on
the device hits 0, the kernel tries to clean up the sysfs files again. This
time the system will error because the files do not exist resulting in an
error.

This patch is a workaround but is needed to enable insertion/removal of the
kernel driver. I think more research into why there is a race condition is
needed. Maybe I did something wrong in my refcounting but this is where I'm
at.

Signed-off-by: Daniel Steger <daniel.steger@xilinx.com>
Reviewed-by: Wendy Liang <wendy.liang@xilinx.com>

drivers/misc/xilinx-ai-engine/ai-engine-internal.h

@@ -633,6 +633,7 @@ struct aie_tile {
 	struct aie_location loc;
 	struct aie_partition *apart;
 	struct device dev;
+	struct attribute_group *attr_grp;
 };
 
 /**
@@ -795,6 +796,7 @@ struct aie_partition {
 	struct aie_resource mem_event_status;
 	struct aie_resource pl_event_status;
 	struct aie_resource l2_mask;
+	struct attribute_group *attr_grp;
 	u32 partition_id;
 	u32 status;
 	u32 cntrflag;
@@ -1041,7 +1043,10 @@ int aie_part_rscmgr_set_tile_broadcast(struct aie_partition *apart,
 				       struct aie_location loc,
 				       enum aie_module_type mod, uint32_t id);
 
-int aie_part_sysfs_init(struct aie_partition *apart);
+int aie_part_sysfs_create_entries(struct aie_partition *apart);
+void aie_part_sysfs_remove_entries(struct aie_partition *apart);
+int aie_tile_sysfs_create_entries(struct aie_tile *atile);
+void aie_tile_sysfs_remove_entries(struct aie_tile *atile);
 ssize_t aie_sysfs_read_handler(struct file *filp, struct kobject *kobj,
 			       struct bin_attribute *attr, char *buf,
 			       loff_t offset, size_t max_size);

drivers/misc/xilinx-ai-engine/ai-engine-part.c

@@ -868,6 +868,14 @@ static int aie_create_tiles(struct aie_partition *apart)
 				put_device(tdev);
 				return ret;
 			}
+			ret = aie_tile_sysfs_create_entries(atile);
+			if (ret) {
+				dev_err(tdev, "failed to create tile sysfs: %d\n",
+					ret);
+				device_del(tdev);
+				put_device(tdev);
+				return ret;
+			}
 			atile++;
 		}
 	}
@@ -999,10 +1007,9 @@ static struct aie_partition *aie_create_partition(struct aie_device *adev,
 		return ERR_PTR(ret);
 	}
 
-	/* Create sysfs interface */
-	ret = aie_part_sysfs_init(apart);
+	ret = aie_part_sysfs_create_entries(apart);
 	if (ret) {
-		dev_err(&apart->dev, "Failed to create sysfs interface.\n");
+		dev_err(&apart->dev, "Failed to create partition sysfs.\n");
 		put_device(dev);
 		return ERR_PTR(ret);
 	}
@@ -1103,6 +1110,7 @@ of_aie_part_probe(struct aie_device *adev, struct device_node *nc)
  */
 static void aie_tile_remove(struct aie_tile *atile)
 {
+	aie_tile_sysfs_remove_entries(atile);
 	device_del(&atile->dev);
 	put_device(&atile->dev);
 }
@@ -1122,6 +1130,8 @@ void aie_part_remove(struct aie_partition *apart)
 	     index++, atile++)
 		aie_tile_remove(atile);
 
+	aie_fpga_free_bridge(apart);
+	aie_part_sysfs_remove_entries(apart);
 	of_node_clear_flag(apart->dev.of_node, OF_POPULATED);
 	device_del(&apart->dev);
 	put_device(&apart->dev);

drivers/misc/xilinx-ai-engine/ai-engine-sysfs.c

@@ -174,13 +174,20 @@ static int aie_tile_sysfs_create(struct aie_tile *atile)
 		if (!attr_grp)
 			return -ENOMEM;
 
+		atile->attr_grp = attr_grp;
+
 		if (attr->num_dev_attrs)
 			attr_grp->attrs = dev_attrs;
 
 		if (attr->num_bin_attrs)
 			attr_grp->bin_attrs = bin_attrs;
 
-		ret = devm_device_add_group(&atile->dev, attr_grp);
+		/* TODO - use the non managed api to create sysfs group.
+		 * This workaround solves an issue where the device_del()
+		 * removes the SYSFS files before the managed create group.
+		 * This results in an error where files cannot be found.
+		 */
+		ret = sysfs_create_group(&atile->dev.kobj, attr_grp);
 		if (ret) {
 			dev_err(&atile->dev,
 				"Failed to add sysfs attributes group\n");
@@ -252,13 +259,20 @@ static int aie_part_sysfs_create(struct aie_partition *apart)
 		if (!attr_grp)
 			return -ENOMEM;
 
+		apart->attr_grp = attr_grp;
+
 		if (attr->num_dev_attrs)
 			attr_grp->attrs = dev_attrs;
 
 		if (attr->num_bin_attrs)
 			attr_grp->bin_attrs = bin_attrs;
 
-		ret = devm_device_add_group(&apart->dev, attr_grp);
+		/* TODO - use the non managed api to create sysfs group.
+		 * This workaround solves an issue where the device_del()
+		 * removes the SYSFS files before the managed create group.
+		 * This results in an error where files cannot be found.
+		 */
+		ret = sysfs_create_group(&apart->dev.kobj, attr_grp);
 		if (ret) {
 			dev_err(&apart->dev,
 				"Failed to add sysfs attributes group\n");
@@ -268,33 +282,53 @@ static int aie_part_sysfs_create(struct aie_partition *apart)
 }
 
 /**
- * aie_part_sysfs_init() - initializes sysfs interface by creating node at tile
- *			   and partition granularity.
+ * aie_part_sysfs_create() - creates sysfs group for partition device.
  * @apart: AI engine partition.
  * @return: 0 for success, error code for failure.
  */
-int aie_part_sysfs_init(struct aie_partition *apart)
+int aie_part_sysfs_create_entries(struct aie_partition *apart)
 {
-	struct aie_tile *atile = apart->atiles;
-	u32 index;
 	int ret;
 
 	ret = aie_part_sysfs_create(apart);
 	if (ret < 0) {
-		dev_err(&apart->dev,
-			"Failed to create partition level sysfs nodes\n");
+		dev_err(&apart->dev, "Failed to create sysfs partition\n");
 		return ret;
 	}
+	return ret;
+}
 
-	for (index = 0; index < apart->range.size.col * apart->range.size.row;
-	     index++) {
-		ret = aie_tile_sysfs_create(atile);
-		if (ret < 0) {
-			dev_err(&atile->dev,
-				"Failed to create tile level sysfs nodes\n");
-			return ret;
-		}
-		atile++;
+/**
+ * aie_tile_sysfs_create() - creates sysfs group for tile device.
+ * @apart: AI engine partition.
+ * @return: 0 for success, error code for failure.
+ */
+int aie_tile_sysfs_create_entries(struct aie_tile *atile)
+{
+	int ret;
+
+	ret = aie_tile_sysfs_create(atile);
+	if (ret < 0) {
+		dev_err(&atile->dev, "Failed to create sysfs tile\n");
+		return ret;
 	}
 	return ret;
 }
+
+/**
+ * aie_part_sysfs_remove() - removes sysfs group from partition device.
+ * @apart: AI engine partition.
+ */
+void aie_part_sysfs_remove_entries(struct aie_partition *apart)
+{
+	sysfs_remove_group(&apart->dev.kobj, apart->attr_grp);
+}
+
+/**
+ * aie_tile_sysfs_remove() - removes sysfs group from tile device.
+ * @atile: AI engine tile.
+ */
+void aie_tile_sysfs_remove_entries(struct aie_tile *atile)
+{
+	sysfs_remove_group(&atile->dev.kobj, atile->attr_grp);
+}