Re: [PATCH v1] media: dvb: Add devm_dvb_register_adapter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

some comments below.

On Fri, Jul 12, 2019 at 04:19:20PM +0200, Marc Gonzalez wrote:
> Add devm* variant for automagic resource release.

S-o-b missing.

> +int devm_dvb_register_adapter(struct device *dev, struct dvb_adapter *adap,
> +		const char *name, struct module *module, short *adapter_nums)
> +{
> +	int v1, v2;
> +
> +	v1 = dvb_register_adapter(adap, name, module, dev, adapter_nums);
> +	if (v1 < 0)
> +		return v1;
> +
> +	v2 = devm_add_action_or_reset(dev, unregister_adapter, adap);
> +	if (v2 < 0)
> +		return v2;
> +
> +	return v1;
> +}
> +EXPORT_SYMBOL(devm_dvb_register_adapter);

What non-negative numbers can dvb_register_adapter and
devm_add_action_or_reset return, and what are their meanings? Why should
devm_dvb_register_adapter return the (non-negative) return value of
dvb_register_adapter?
(I really don't know, because I'm not familiar with the media/DVB subsystem)

*If* the non-negative return values don't matter, I'd simplify the code
to something like this:

	int res;

	res = dvb_register_adapter(adap, name, module, dev, adapter_nums);
	if (res < 0)
		return res;

	res = devm_add_action_or_reset(dev, unregister_adapter, adap);
	if (res < 0)
		return res;

	return 0;

(or even 'return devm_add_action_or_reset(...)' directly)


> +int devm_dvb_register_adapter(struct device *dev, struct dvb_adapter *adap,
> +		const char *name, struct module *module, short *adapter_nums);

I think this function should also be added to
Documentation/driver-model/devres.rst (previously called devres.txt),
considering that most (or all?) devm_ functions are listed there.


Thanks,
Jonthan Neuschäfer

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux