Re: [PATCH v6 06/11] cec: add HDMI CEC framework

Sean Young
2015-05-13T11:15:50Z

On Mon, May 04, 2015 at 07:32:59PM +0200, Kamil Debski wrote:
> From: Hans Verkuil <hansverk@cisco.com>
>
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
>
> Signed-off-by: Hans Verkuil <hansverk@cisco.com>

-snip-

> +int cec_create_adapter(struct cec_adapter *adap, const char *name, u32 caps)
> +{
> + int res = 0;
> +
> + adap->state = CEC_ADAP_STATE_DISABLED;
> + adap->name = name;
> + adap->phys_addr = 0xffff;
> + adap->capabilities = caps;
> + adap->version = CEC_VERSION_1_4;
> + adap->sequence = 0;
> + mutex_init(&adap->lock);
> + adap->kthread = kthread_run(cec_thread_func, adap, name);
> + init_waitqueue_head(&adap->kthread_waitq);
> + init_waitqueue_head(&adap->waitq);
> + if (IS_ERR(adap->kthread)) {
> + pr_err("cec-%s: kernel_thread() failed\n", name);
> + return PTR_ERR(adap->kthread);
> + }
> + if (caps) {
> + res = cec_devnode_register(&adap->devnode, adap->owner);
> + if (res)
> + kthread_stop(adap->kthread);
> + }
> + adap->recv_notifier = cec_receive_notify;
> +
> + /* Prepare the RC input device */
> + adap->rc = rc_allocate_device();
> + if (!adap->rc) {
> + pr_err("cec-%s: failed to allocate memory for rc_dev\n", name);
> + cec_devnode_unregister(&adap->devnode);
> + kthread_stop(adap->kthread);
> + return -ENOMEM;
> + }
> +
> + snprintf(adap->input_name, sizeof(adap->input_name), "RC for %s", name);
> + snprintf(adap->input_phys, sizeof(adap->input_phys), "%s/input0", name);
> + strncpy(adap->input_drv, name, sizeof(adap->input_drv));
> +
> + adap->rc->input_name = adap->input_name;
> + adap->rc->input_phys = adap->input_phys;
> + adap->rc->dev.parent = &adap->devnode.dev;
> + adap->rc->driver_name = adap->input_drv;
> + adap->rc->driver_type = RC_DRIVER_CEC;
> + adap->rc->allowed_protocols = RC_BIT_CEC;
> + adap->rc->priv = adap;
> + adap->rc->map_name = RC_MAP_CEC;
> + adap->rc->timeout = MS_TO_NS(100);
> +

rc->input_id is not populated. It would be nice if input_phys has some
resemblance to a physical path (like the output of usb_make_path() if it
is a usb device).

> + res = rc_register_device(adap->rc);
> +
> + if (res) {
> + pr_err("cec-%s: failed to prepare input device\n", name);
> + cec_devnode_unregister(&adap->devnode);
> + rc_free_device(adap->rc);
> + kthread_stop(adap->kthread);
> + }
> +
> + return res;
> +}
> +EXPORT_SYMBOL_GPL(cec_create_adapter);
> +
> +void cec_delete_adapter(struct cec_adapter *adap)
> +{
> + if (adap->kthread == NULL)
> + return;
> + kthread_stop(adap->kthread);
> + if (adap->kthread_config)
> + kthread_stop(adap->kthread_config);
> + adap->state = CEC_ADAP_STATE_DISABLED;
> + if (cec_devnode_is_registered(&adap->devnode))
> + cec_devnode_unregister(&adap->devnode);

I think you're missing a rc_unregister_device() here.

> +}
> +EXPORT_SYMBOL_GPL(cec_delete_adapter);

Sean