On 30/08/2024 11:23, songchai wrote:

Add driver to support Coresight device TGU (Trigger Generation Unit).
TGU is a Data Engine which can be utilized to sense a plurality of
signals and create a trigger into the CTI or generate interrupts to
processors. Add probe/enable/disable functions for tgu.

Signed-off-by: songchai <redacted>

...

+
+static struct attribute *tgu_common_attrs[] = {
+	&dev_attr_enable_tgu.attr,
+	NULL,
+};
+
+static struct attribute_group tgu_common_grp = {

Not const?

+	.attrs = tgu_common_attrs,
+	NULL,
+};
+
+static const struct attribute_group *tgu_attr_groups[] = {
+	&tgu_common_grp,
+	NULL,
+};
+
+static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	int ret = 0;
+	struct device *dev = &adev->dev;
+	struct coresight_platform_data *pdata;
+	struct tgu_drvdata *drvdata;
+	struct coresight_desc desc = { 0 };

Code is quite mixed here... Bring some order - declarations with and
without assignments.

+
+	desc.name = coresight_alloc_device_name(&tgu_devs, dev);
+	if (!desc.name)
+		return -ENOMEM;
+
+	pdata = coresight_get_platform_data(dev);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+
+	adev->dev.platform_data = pdata;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->dev = &adev->dev;
+	dev_set_drvdata(dev, drvdata);
+
+	drvdata->base = devm_ioremap_resource(dev, &adev->res);
+	if (!drvdata->base)
+		return -ENOMEM;
+
+	spin_lock_init(&drvdata->spinlock);
+
+	drvdata->enable = false;
+	desc.type = CORESIGHT_DEV_TYPE_HELPER;
+	desc.pdata = adev->dev.platform_data;
+	desc.dev = &adev->dev;
+	desc.ops = &tgu_ops;
+	desc.groups = tgu_attr_groups;
+
+	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev)) {
+		ret = PTR_ERR(drvdata->csdev);
+		goto err;
+	}
+
+	pm_runtime_put(&adev->dev);
+	dev_dbg(dev, "TGU initialized\n");

Drop, useless. Kernel provides you already ways to know probe status.

+	return 0;
+err:
+	pm_runtime_put(&adev->dev);
+	return ret;
+}
+
+static struct amba_id tgu_ids[] = {

Not const?

+	{
+		.id = 0x0003b999,
+		.mask = 0x0003ffff,
+		.data = "TGU",
+	},
+	{ 0, 0 },
+};

No module device table?

+
+static struct amba_driver tgu_driver = {
+	.drv = {
+			.name = "coresight-tgu",
+			.owner = THIS_MODULE,

Please drop. Also one-less indentation.

+			.suppress_bind_attrs = true,
+		},
+	.probe = tgu_probe,
+	.id_table = tgu_ids,
+};
+
+module_amba_driver(tgu_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("CoreSight TGU driver");

Best regards,
Krzysztof