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 <quic_songchai@xxxxxxxxxxx>
...
> +
> +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
