[PATCH v1 0/8] media: cec: core: Ad-hoc refactoring

[PATCH v1 0/8] media: cec: core: Ad-hoc refactoring

[Andy Shevchenko]

Refactor cec-core to use modern headers and definitions and fix
some minor issues. No functional change intended.

Also get rid of MS_TO_US() use from "cec", leave it to be "rc"-only
custom macro, which can be dropped later on as well.

Andy Shevchenko (8):
  media: cec: core: consolidate error path in cec_allocate_adapter()
  media: cec: core: add missing mutex_destroy to error path and remove
  media: cec: core: Use DEFINE_SHOW_STORE_ATTRIBUTE() helper for debugfs
  media: cec: core: Don't use "proxy" headers
  media: cec: core: Add pr_fmt()
  media: cec: core: Consistently use CEC_NAME where it matters
  media: cec: core: Use predefined time multiplier
  media: cec: seco: Use predefined time multiplier

 drivers/media/cec/core/cec-core.c          | 92 +++++++++++++---------
 drivers/media/cec/platform/seco/seco-cec.c |  5 +-
 2 files changed, 59 insertions(+), 38 deletions(-)

-- 
2.50.1

[PATCH v1 1/8] media: cec: core: consolidate error path in cec_allocate_adapter()

[Andy Shevchenko]

Consolidate error path in cec_allocate_adapter() so next changes
will be less verbose.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 90a98f322f5e..55011e555d35 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -267,8 +267,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	if (IS_ERR(adap->kthread)) {
 		pr_err("cec-%s: kernel_thread() failed\n", name);
 		res = PTR_ERR(adap->kthread);
-		kfree(adap);
-		return ERR_PTR(res);
+		goto err_free_adap;
 	}
 
 #ifdef CONFIG_MEDIA_CEC_RC
@@ -281,8 +280,8 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 		pr_err("cec-%s: failed to allocate memory for rc_dev\n",
 		       name);
 		kthread_stop(adap->kthread);
-		kfree(adap);
-		return ERR_PTR(-ENOMEM);
+		res = -ENOMEM;
+		goto err_free_adap;
 	}
 
 	snprintf(adap->input_phys, sizeof(adap->input_phys),
@@ -301,6 +300,10 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	adap->rc->timeout = MS_TO_US(550);
 #endif
 	return adap;
+
+err_free_adap:
+	kfree(adap);
+	return ERR_PTR(res);
 }
 EXPORT_SYMBOL_GPL(cec_allocate_adapter);
 
-- 
2.50.1

[PATCH v1 2/8] media: cec: core: add missing mutex_destroy to error path and remove

[Andy Shevchenko]

Add missing mutex_destroy() call to error path and remove to properly
clean up the mutex initialized during adapter allocation. Ensure proper
resource cleanup and follows kernel practices.

Found by code review.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 55011e555d35..0c89fac58349 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -302,6 +302,11 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	return adap;
 
 err_free_adap:
+	mutex_destroy(&adap->devnode.lock);
+	mutex_destroy(&adap->devnode.lock_fhs);
+
+	mutex_destroy(&adap->lock);
+
 	kfree(adap);
 	return ERR_PTR(res);
 }
@@ -388,14 +393,23 @@ void cec_delete_adapter(struct cec_adapter *adap)
 {
 	if (IS_ERR_OR_NULL(adap))
 		return;
+
 	if (adap->kthread_config)
 		kthread_stop(adap->kthread_config);
 	kthread_stop(adap->kthread);
+
 	if (adap->ops->adap_free)
 		adap->ops->adap_free(adap);
+
 #ifdef CONFIG_MEDIA_CEC_RC
 	rc_free_device(adap->rc);
 #endif
+
+	mutex_destroy(&adap->devnode.lock);
+	mutex_destroy(&adap->devnode.lock_fhs);
+
+	mutex_destroy(&adap->lock);
+
 	kfree(adap);
 }
 EXPORT_SYMBOL_GPL(cec_delete_adapter);
-- 
2.50.1

[PATCH v1 3/8] media: cec: core: Use DEFINE_SHOW_STORE_ATTRIBUTE() helper for debugfs

[Andy Shevchenko]

Use DEFINE_SHOW_STORE_ATTRIBUTE() helper for read-write file to reduce some
duplicated code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 0c89fac58349..674bcb6450dc 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -205,19 +205,7 @@ static int cec_error_inj_show(struct seq_file *sf, void *unused)
 
 	return call_op(adap, error_inj_show, sf);
 }
-
-static int cec_error_inj_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, cec_error_inj_show, inode->i_private);
-}
-
-static const struct file_operations cec_error_inj_fops = {
-	.open = cec_error_inj_open,
-	.write = cec_error_inj_write,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(cec_error_inj);
 #endif
 
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
-- 
2.50.1

[PATCH v1 4/8] media: cec: core: Don't use "proxy" headers

[Andy Shevchenko]

Update header inclusions to follow IWYU (Include What You Use)
principle.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 674bcb6450dc..1f12fb0caca7 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -5,16 +5,30 @@
  * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
  */
 
+#include <linux/bitops.h>
+#include <linux/bug.h>
+#include <linux/cdev.h>
+#include <linux/container_of.h>
 #include <linux/debugfs.h>
-#include <linux/errno.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/fs.h>
 #include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/kmod.h>
-#include <linux/mm.h>
+#include <linux/kobject.h>
+#include <linux/kthread.h>
+#include <linux/list.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/printk.h>
+#include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/sprintf.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/wait.h>
+
+#include <asm/page.h>
 
 #include "cec-priv.h"
 
-- 
2.50.1

[PATCH v1 5/8] media: cec: core: Add pr_fmt()

[Andy Shevchenko]

Several prints inconsistently use cec: or cec-%s: or nothing. To make
it clear which prints come from cec-core.c, add a pr_fmt() macro.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 1f12fb0caca7..dc05c5d7b2e3 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -5,6 +5,8 @@
  * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
  */
 
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
 #include <linux/bitops.h>
 #include <linux/bug.h>
 #include <linux/cdev.h>
@@ -107,7 +109,7 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode,
 	minor = find_first_zero_bit(cec_devnode_nums, CEC_NUM_DEVICES);
 	if (minor == CEC_NUM_DEVICES) {
 		mutex_unlock(&cec_devnode_lock);
-		pr_err("could not get a free minor\n");
+		pr_err("Could not get a free minor\n");
 		return -ENFILE;
 	}
 
@@ -130,7 +132,7 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode,
 	ret = cdev_device_add(&devnode->cdev, &devnode->dev);
 	if (ret) {
 		devnode->registered = false;
-		pr_err("%s: cdev_device_add failed\n", __func__);
+		pr_err("cdev_device_add() failed\n");
 		goto clr_bit;
 	}
 
@@ -267,7 +269,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 
 	adap->kthread = kthread_run(cec_thread_func, adap, "cec-%s", name);
 	if (IS_ERR(adap->kthread)) {
-		pr_err("cec-%s: kernel_thread() failed\n", name);
+		pr_err("%s: kernel_thread() failed\n", name);
 		res = PTR_ERR(adap->kthread);
 		goto err_free_adap;
 	}
@@ -279,8 +281,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	/* Prepare the RC input device */
 	adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!adap->rc) {
-		pr_err("cec-%s: failed to allocate memory for rc_dev\n",
-		       name);
+		pr_err("%s: failed to allocate memory for rc_dev\n", name);
 		kthread_stop(adap->kthread);
 		res = -ENOMEM;
 		goto err_free_adap;
@@ -336,8 +337,7 @@ int cec_register_adapter(struct cec_adapter *adap,
 		res = rc_register_device(adap->rc);
 
 		if (res) {
-			pr_err("cec-%s: failed to prepare input device\n",
-			       adap->name);
+			pr_err("%s: failed to prepare input device\n", adap->name);
 			rc_free_device(adap->rc);
 			adap->rc = NULL;
 			return res;
@@ -424,14 +424,14 @@ static int __init cec_devnode_init(void)
 	int ret = alloc_chrdev_region(&cec_dev_t, 0, CEC_NUM_DEVICES, CEC_NAME);
 
 	if (ret < 0) {
-		pr_warn("cec: unable to allocate major\n");
+		pr_warn("Unable to allocate major\n");
 		return ret;
 	}
 
 #ifdef CONFIG_DEBUG_FS
 	top_cec_dir = debugfs_create_dir("cec", NULL);
 	if (IS_ERR_OR_NULL(top_cec_dir)) {
-		pr_warn("cec: Failed to create debugfs cec dir\n");
+		pr_warn("Failed to create debugfs cec dir\n");
 		top_cec_dir = NULL;
 	}
 #endif
@@ -440,7 +440,7 @@ static int __init cec_devnode_init(void)
 	if (ret < 0) {
 		debugfs_remove_recursive(top_cec_dir);
 		unregister_chrdev_region(cec_dev_t, CEC_NUM_DEVICES);
-		pr_warn("cec: bus_register failed\n");
+		pr_warn("bus_register() failed\n");
 		return -EIO;
 	}
 
-- 
2.50.1

[PATCH v1 6/8] media: cec: core: Consistently use CEC_NAME where it matters

[Andy Shevchenko]

When create a debugfs folder or device nodes we use actual subsystem
name. But in some cases it's hardcoded, while in other it uses predefined
string literal. Make it consistently use CEC_NAME.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index dc05c5d7b2e3..1b6f27a552db 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -120,13 +120,13 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode,
 	devnode->dev.bus = &cec_bus_type;
 	devnode->dev.devt = MKDEV(MAJOR(cec_dev_t), minor);
 	devnode->dev.release = cec_devnode_release;
-	dev_set_name(&devnode->dev, "cec%d", devnode->minor);
+	dev_set_name(&devnode->dev, "%s%d", CEC_NAME, devnode->minor);
 	device_initialize(&devnode->dev);
 
 	/* Part 2: Initialize and register the character device */
 	cdev_init(&devnode->cdev, &cec_devnode_fops);
 	devnode->cdev.owner = owner;
-	kobject_set_name(&devnode->cdev.kobj, "cec%d", devnode->minor);
+	kobject_set_name(&devnode->cdev.kobj, "%s%d", CEC_NAME, devnode->minor);
 
 	devnode->registered = true;
 	ret = cdev_device_add(&devnode->cdev, &devnode->dev);
@@ -267,7 +267,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	mutex_init(&adap->devnode.lock_fhs);
 	mutex_init(&adap->devnode.lock);
 
-	adap->kthread = kthread_run(cec_thread_func, adap, "cec-%s", name);
+	adap->kthread = kthread_run(cec_thread_func, adap, "%s-%s", CEC_NAME, name);
 	if (IS_ERR(adap->kthread)) {
 		pr_err("%s: kernel_thread() failed\n", name);
 		res = PTR_ERR(adap->kthread);
@@ -429,9 +429,9 @@ static int __init cec_devnode_init(void)
 	}
 
 #ifdef CONFIG_DEBUG_FS
-	top_cec_dir = debugfs_create_dir("cec", NULL);
+	top_cec_dir = debugfs_create_dir(CEC_NAME, NULL);
 	if (IS_ERR_OR_NULL(top_cec_dir)) {
-		pr_warn("Failed to create debugfs cec dir\n");
+		pr_warn("Failed to create debugfs " CEC_NAME " dir\n");
 		top_cec_dir = NULL;
 	}
 #endif
-- 
2.50.1

[PATCH v1 7/8] media: cec: core: Use predefined time multiplier

[Andy Shevchenko]

Instead of relying on custom MS_TO_US() macro, use existing time
multiplier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/core/cec-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 1b6f27a552db..a67c4283fbba 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/sprintf.h>
 #include <linux/string.h>
+#include <linux/time.h>
 #include <linux/types.h>
 #include <linux/wait.h>
 
@@ -300,7 +301,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	adap->rc->allowed_protocols = RC_PROTO_BIT_CEC;
 	adap->rc->priv = adap;
 	adap->rc->map_name = RC_MAP_CEC;
-	adap->rc->timeout = MS_TO_US(550);
+	adap->rc->timeout = 550 * USEC_PER_MSEC;
 #endif
 	return adap;
 
-- 
2.50.1

[PATCH v1 8/8] media: cec: seco: Use predefined time multiplier

[Andy Shevchenko]

Instead of relying on custom MS_TO_US() macro, use existing time
multiplier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/cec/platform/seco/seco-cec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/platform/seco/seco-cec.c b/drivers/media/cec/platform/seco/seco-cec.c
index 97ed9654c78a..654c70503dd5 100644
--- a/drivers/media/cec/platform/seco/seco-cec.c
+++ b/drivers/media/cec/platform/seco/seco-cec.c
@@ -7,14 +7,15 @@
  * Copyright (C) 2018, Aidilab Srl.
  */
 
-#include <linux/module.h>
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
+#include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/time.h>
 
 /* CEC Framework */
 #include <media/cec-notifier.h>
@@ -356,7 +357,7 @@ static int secocec_ir_probe(void *priv)
 	cec->ir->allowed_protocols = RC_PROTO_BIT_RC5;
 	cec->ir->priv = cec;
 	cec->ir->map_name = RC_MAP_HAUPPAUGE;
-	cec->ir->timeout = MS_TO_US(100);
+	cec->ir->timeout = 100 * USEC_PER_MSEC;
 
 	/* Clear the status register */
 	status = smb_rd16(SECOCEC_STATUS_REG_1, &val);
-- 
2.50.1