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

[Sumit Kumar]

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