Re: [PATCH v5 1/3] bus: mhi: host: clients: Add loopback driver with sysfs interface

[Sumit Kumar <sumit.kumar@oss.qualcomm.com>]

On 8/18/2026 2:20 PM, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Aug 17, 2026 at 03:44:34PM +0530, Sumit Kumar wrote:
>> +#include <linux/mhi.h>
>> +#include <linux/mod_devicetable.h>
> Please rely on <linux/mhi.h> to provide mhi_device_id and drop the
> inclusion for <linux/mod_devicetable.h>. The latter header is ugly and
> should go away soon.
thanks, Will rebase and fix it
>
>> [...]
>> +static struct attribute *mhi_loopback_attrs[] = {
>> +	&dev_attr_tre_size.attr,
>> +	&dev_attr_max_tre_size.attr,
>> +	&dev_attr_num_tre.attr,
>> +	&dev_attr_start.attr,
>> +	NULL,
> Please no , after sentinel entries.
Acknowledged, will remove it.
>
>> +};
>> +
>> +static const struct attribute_group mhi_loopback_group = {
>> +	.attrs = mhi_loopback_attrs,
>> +};
>> +
>> +static int mhi_loopback_probe(struct mhi_device *mhi_dev,
>> +			      const struct mhi_device_id *id)
>> +{
>> +	struct mhi_loopback *loopback;
>> +	int ret;
>> +
>> +	loopback = devm_kzalloc(&mhi_dev->dev, sizeof(*loopback), GFP_KERNEL);
>> +	if (!loopback)
>> +		return -ENOMEM;
>> +
>> +	loopback->mdev = mhi_dev;
>> +	loopback->tre_size = MHI_LOOPBACK_DEFAULT_TRE_SIZE;
>> +	loopback->num_tre = MHI_LOOPBACK_DEFAULT_NUM_TRE;
>> +
>> +	mutex_init(&loopback->lb_mutex);
>> +	init_completion(&loopback->comp);
>> +
>> +	dev_set_drvdata(&mhi_dev->dev, loopback);
>> +
>> +	ret = mhi_prepare_for_transfer(mhi_dev);
>> +	if (ret) {
>> +		dev_err(&mhi_dev->dev, "Failed to prepare for transfers: %d\n", ret);
> dev_err_probe() please.
Sure.
>
>> +		return ret;
>> +	}
>> +
>> +	ret = sysfs_create_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
>> +	if (ret) {
>> +		dev_err(&mhi_dev->dev, "Failed to create sysfs attributes: %d\n", ret);
>> +		mhi_unprepare_from_transfer(mhi_dev);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void mhi_loopback_remove(struct mhi_device *mhi_dev)
>> +{
>> +	/* Blocks until any in-progress store() has returned */
>> +	sysfs_remove_group(&mhi_dev->dev.kobj, &mhi_loopback_group);
>> +	mhi_unprepare_from_transfer(mhi_dev);
>> +}
>> +
>> +static const struct mhi_device_id mhi_loopback_id_table[] = {
>> +	{ .chan = "LOOPBACK"},
>> +	{}
> { } (i.e. a space between the curly braces) is the more common way to
> write that. Please also add a space before the closing } in the line
> above.
Will fix this too.
>
>> +};
>> +MODULE_DEVICE_TABLE(mhi, mhi_loopback_id_table);
> Best regards
> Uwe