[1/1] media: dvb_dummy_tuner: implement driver skeleton
Commit Message
From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
Implement the skeleton for the tuner driver in a separate file.
Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com>
---
drivers/media/tuners/Kconfig | 7 ++
drivers/media/tuners/Makefile | 1 +
drivers/media/tuners/dvb_dummy_tuner.c | 153 +++++++++++++++++++++++++
drivers/media/tuners/dvb_dummy_tuner.h | 20 ++++
4 files changed, 181 insertions(+)
create mode 100644 drivers/media/tuners/dvb_dummy_tuner.c
create mode 100644 drivers/media/tuners/dvb_dummy_tuner.h
Comments
On Thu, Jan 09, 2020 at 12:24:08PM -0300, Daniel W. S. Almeida wrote:
> +static int dvb_dummy_tuner_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret = 0;
> + struct dvb_dummy_tuner_config *config = client->dev.platform_data;
> + struct dvb_frontend *fe = config->fe;
> + struct dvb_dummy_tuner_dev *tuner_dev = NULL;
> +
> + tuner_dev = kzalloc(sizeof(*tuner_dev), GFP_KERNEL);
> + if (!tuner_dev) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + i2c_set_clientdata(client, tuner_dev);
> + tuner_dev->fe = config->fe;
> +
> + memcpy(&fe->ops.tuner_ops,
> + &dvb_dummy_tuner_ops,
> + sizeof(struct dvb_tuner_ops));
> +
> + fe->tuner_priv = client;
> +
> + pr_debug("%s: Successfully probed %s\n", __func__, client->name);
As you are a driver, you should never need to call any pr_* calls,
instead use dev_*(). For this, you can use dev_dbg(), but really, why
is that even needed except for your debugging bringup. And for that,
you can use ftrace, right? So no need for any printing of anything
here.
> + return ret;
> +
> +err:
> + pr_err("%s: failed\n", __func__);
Again, dev_err() would be proper, but there's no need for any error
message here.
Don't you need to register the tuner ops with something in this
function?
> + return ret;
> +}
> +
> +static int dvb_dummy_tuner_i2c_remove(struct i2c_client *client)
> +{
> + struct dvb_dummy_tuner_dev *tuner_dev = i2c_get_clientdata(client);
> + struct dvb_frontend *fe = tuner_dev->fe;
> +
> + memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
> + fe->tuner_priv = NULL;
> + kfree(tuner_dev);
> +
Don't you need to unregister the tuner ops in here?
thanks,
greg k-h
Em Thu, 9 Jan 2020 12:24:08 -0300
"Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:
> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
>
> Implement the skeleton for the tuner driver in a separate file.
>
> Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com>
> ---
> drivers/media/tuners/Kconfig | 7 ++
> drivers/media/tuners/Makefile | 1 +
> drivers/media/tuners/dvb_dummy_tuner.c | 153 +++++++++++++++++++++++++
> drivers/media/tuners/dvb_dummy_tuner.h | 20 ++++
> 4 files changed, 181 insertions(+)
> create mode 100644 drivers/media/tuners/dvb_dummy_tuner.c
> create mode 100644 drivers/media/tuners/dvb_dummy_tuner.h
>
> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
> index e104bb7766e1..efa1591fe0ae 100644
> --- a/drivers/media/tuners/Kconfig
> +++ b/drivers/media/tuners/Kconfig
> @@ -296,4 +296,11 @@ config MEDIA_TUNER_QM1D1B0004
> default m if !MEDIA_SUBDRV_AUTOSELECT
> help
> Sharp QM1D1B0004 ISDB-S tuner driver.
> +
> +config MEDIA_TUNER_DVB_DUMMY_TUNER
> + tristate "Dummy tuner"
> + depends on MEDIA_SUPPORT && I2C
> + default m if !MEDIA_SUBDRV_AUTOSELECT
Actually, this is a dummy driver. It should not be auto-selected, as no
real hardware depends on it. We don't want production distros to come with
those test drivers.
When we add a bridge driver (let's say we call it DVB_DUMMY_BRIDGE), then
what we could do here would be something like:
depends on DVB_DUMMY_BRIDGE
default y
Cheers,
Mauro
Hi Daniel,
On 1/9/20 8:24 AM, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
>
> Implement the skeleton for the tuner driver in a separate file.
Please add more detail here on what this driver does. What this
driver is used for and so on.
>
> Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com>
> ---
> drivers/media/tuners/Kconfig | 7 ++
> drivers/media/tuners/Makefile | 1 +
> drivers/media/tuners/dvb_dummy_tuner.c | 153 +++++++++++++++++++++++++
> drivers/media/tuners/dvb_dummy_tuner.h | 20 ++++
> 4 files changed, 181 insertions(+)
> create mode 100644 drivers/media/tuners/dvb_dummy_tuner.c
> create mode 100644 drivers/media/tuners/dvb_dummy_tuner.h
>
> diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
> index e104bb7766e1..efa1591fe0ae 100644
> --- a/drivers/media/tuners/Kconfig
> +++ b/drivers/media/tuners/Kconfig
> @@ -296,4 +296,11 @@ config MEDIA_TUNER_QM1D1B0004
> default m if !MEDIA_SUBDRV_AUTOSELECT
> help
> Sharp QM1D1B0004 ISDB-S tuner driver.
> +
> +config MEDIA_TUNER_DVB_DUMMY_TUNER
> + tristate "Dummy tuner"
> + depends on MEDIA_SUPPORT && I2C
> + default m if !MEDIA_SUBDRV_AUTOSELECT
> + help
> + Dummy tuner driver for common TV standards.
Samehere. Give more details on what this driver does. Add enough detail
to help users decide why they should enable it or not.
> endmenu
> diff --git a/drivers/media/tuners/Makefile b/drivers/media/tuners/Makefile
> index 7b4f8423501e..f98de1cf2e19 100644
> --- a/drivers/media/tuners/Makefile
> +++ b/drivers/media/tuners/Makefile
> @@ -44,5 +44,6 @@ obj-$(CONFIG_MEDIA_TUNER_QM1D1C0042) += qm1d1c0042.o
> obj-$(CONFIG_MEDIA_TUNER_QM1D1B0004) += qm1d1b0004.o
> obj-$(CONFIG_MEDIA_TUNER_M88RS6000T) += m88rs6000t.o
> obj-$(CONFIG_MEDIA_TUNER_TDA18250) += tda18250.o
> +obj-$(CONFIG_MEDIA_TUNER_DVB_DUMMY_TUNER) += dvb_dummy_tuner.o
>
> ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
> diff --git a/drivers/media/tuners/dvb_dummy_tuner.c b/drivers/media/tuners/dvb_dummy_tuner.c
> new file mode 100644
> index 000000000000..63d2e47d4739
> --- /dev/null
> +++ b/drivers/media/tuners/dvb_dummy_tuner.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for Dummy Frontend
> + *
> + * Written by Daniel W. S. Almeida <dwlsalmeida@gmail.com>
General practice is the following.
Copyright (c) 2020 Daniel W. S. Almeida <dwlsalmeida@gmail.com>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <media/dvb_frontend.h>
> +#include "dvb_dummy_tuner.h"
> +
> +struct dvb_dummy_tuner_dev {
> + struct dvb_frontend *fe;
> +};
> +
> +static void dvb_dummy_tuner_release(struct dvb_frontend *fe)
> +{
> +}
> +
> +static int dvb_dummy_tuner_init(struct dvb_frontend *fe)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_sleep(struct dvb_frontend *fe)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_suspend(struct dvb_frontend *fe)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_resume(struct dvb_frontend *fe)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_set_params(struct dvb_frontend *fe)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_set_config(struct dvb_frontend *fe, void *priv_cfg)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_get_frequency(struct dvb_frontend *fe,
> + u32 *frequency)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_get_bandwidth(struct dvb_frontend *fe,
> + u32 *bandwidth)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_get_if_frequency(struct dvb_frontend *fe,
> + u32 *frequency)
> +{
> + return 0;
> +}
> +
> +static int dvb_dummy_tuner_get_status(struct dvb_frontend *fe, u32 *status)
> +{
> + return 0;
> +}
> +
> +static const struct dvb_tuner_ops dvb_dummy_tuner_ops = {
> + .release = dvb_dummy_tuner_release,
> + .init = dvb_dummy_tuner_init,
> + .sleep = dvb_dummy_tuner_sleep,
> + .suspend = dvb_dummy_tuner_suspend,
> + .resume = dvb_dummy_tuner_resume,
> + .set_params = dvb_dummy_tuner_set_params,
> + .set_config = dvb_dummy_tuner_set_config,
> + .get_bandwidth = dvb_dummy_tuner_get_bandwidth,
> + .get_frequency = dvb_dummy_tuner_get_frequency,
> + .get_if_frequency = dvb_dummy_tuner_get_if_frequency,
> + .get_status = dvb_dummy_tuner_get_status,
> +};
> +
> +static const struct i2c_device_id dvb_dummy_tuner_i2c_id_table[] = {
> + {"dvb_dummy_tuner", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, dvb_dummy_tuner_i2c_id_table);
> +
> +static int dvb_dummy_tuner_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret = 0;
> + struct dvb_dummy_tuner_config *config = client->dev.platform_data;
> + struct dvb_frontend *fe = config->fe;
> + struct dvb_dummy_tuner_dev *tuner_dev = NULL;
> +
> + tuner_dev = kzalloc(sizeof(*tuner_dev), GFP_KERNEL);
> + if (!tuner_dev) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + i2c_set_clientdata(client, tuner_dev);
> + tuner_dev->fe = config->fe;
> +
> + memcpy(&fe->ops.tuner_ops,
> + &dvb_dummy_tuner_ops,
> + sizeof(struct dvb_tuner_ops));
> +
> + fe->tuner_priv = client;
> +
> + pr_debug("%s: Successfully probed %s\n", __func__, client->name);
> + return ret;
> +
> +err:
> + pr_err("%s: failed\n", __func__);
> + return ret;
> +}
> +
> +static int dvb_dummy_tuner_i2c_remove(struct i2c_client *client)
> +{
> + struct dvb_dummy_tuner_dev *tuner_dev = i2c_get_clientdata(client);
> + struct dvb_frontend *fe = tuner_dev->fe;
> +
> + memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
> + fe->tuner_priv = NULL;
> + kfree(tuner_dev);
> +
> + return 0;
> +}
> +
> +static struct i2c_driver dvb_dummy_tuner_i2c_driver = {
> + .driver = {
> + .name = "dvb_dummy_tuner",
> + .suppress_bind_attrs = true,
> + },
> + .probe = dvb_dummy_tuner_i2c_probe,
> + .remove = dvb_dummy_tuner_i2c_remove,
> + .id_table = dvb_dummy_tuner_i2c_id_table,
> +};
> +module_i2c_driver(dvb_dummy_tuner_i2c_driver);
> +
> +MODULE_DESCRIPTION("DVB Dummy Tuner");
> +MODULE_AUTHOR("Daniel W. S. Almeida");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/tuners/dvb_dummy_tuner.h b/drivers/media/tuners/dvb_dummy_tuner.h
> new file mode 100644
> index 000000000000..fde7628258fa
> --- /dev/null
> +++ b/drivers/media/tuners/dvb_dummy_tuner.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * DVB Dummy Tuner
> + *
Please add details about the driver similar to the .c file.
> + */
> +#ifndef __TUNER_DVB_DUMMY_TUNER_H__
> +#define __TUNER_DVB_DUMMY_TUNER_H__
> +
> +#include <media/dvb_frontend.h>
> +/**
> + * struct dvb_dummy_tuner_config - configuration parameters for the dummy tuner
> + *
> + * @fe:
> + * frontend returned by driver
> + */
> +struct dvb_dummy_tuner_config {
> + struct dvb_frontend *fe;
> +};
> +
> +#endif /*__TUNER_DVB_DUMMY_TUNER_H__ */
>
Looks like there is more code to be added to register/unregister
thanks,
-- Shuah
Hi Greg! Thanks for chiming in.
> As you are a driver, you should never need to call any pr_* calls,
> instead use dev_*(). For this, you can use dev_dbg(), but really, why
> is that even needed except for your debugging bringup. And for that,
> you can use ftrace, right? So no need for any printing of anything
> here.
> Again, dev_err() would be proper, but there's no need for any error
> message here.
Let's take these out in v2 then.
> Don't you need to register the tuner ops with something in this
> function?
> Don't you need to unregister the tuner ops in here?
It is my understanding that bridge drivers are the ones responsible for
this. For instance, I don't see this with either xc4000.c, xc5000.c or
mt2060.c. I could be wrong, though. Maybe Mauro could clarify this?
What I did miss in this patch was an attach function. Let's also add
this in v2.
Thanks again,
- Daniel.
Hi Mauro!
> Actually, this is a dummy driver. It should not be auto-selected, as no
> real hardware depends on it. We don't want production distros to come with
> those test drivers.
Let's address this in v2.
Thanks.
Hi Shuah, thank you for you input!
>
> Looks like there is more code to be added to register/unregister
Just to be clear, attach() is missing, but register / unregister belong
in the bridge driver. Is this correct?
>
> Please add more detail here on what this driver does. What this
> driver is used for and so on.
>
>
> Samehere. Give more details on what this driver does. Add enough detail
> to help users decide why they should enable it or not.
>
> General practice is the following.
>
> Copyright (c) 2020 Daniel W. S. Almeida <dwlsalmeida@gmail.com>
> Please add details about the driver similar to the .c file.
OK.
Thanks
- Daniel
On 1/9/20 2:00 PM, Daniel W. S. Almeida wrote:
> Hi Shuah, thank you for you input!
>
>
>>
>> Looks like there is more code to be added to register/unregister
>
> Just to be clear, attach() is missing, but register / unregister belong
> in the bridge driver. Is this correct?
>
>
Right attach/detach is what I was thinking.
>>
>> Please add more detail here on what this driver does. What this
>> driver is used for and so on.
>>
>>
>> Samehere. Give more details on what this driver does. Add enough detail
>> to help users decide why they should enable it or not.
>> General practice is the following.
>>
>> Copyright (c) 2020 Daniel W. S. Almeida <dwlsalmeida@gmail.com>
>
>> Please add details about the driver similar to the .c file.
>
> OK.
>
thanks,
-- Shuah
@@ -296,4 +296,11 @@ config MEDIA_TUNER_QM1D1B0004
default m if !MEDIA_SUBDRV_AUTOSELECT
help
Sharp QM1D1B0004 ISDB-S tuner driver.
+
+config MEDIA_TUNER_DVB_DUMMY_TUNER
+ tristate "Dummy tuner"
+ depends on MEDIA_SUPPORT && I2C
+ default m if !MEDIA_SUBDRV_AUTOSELECT
+ help
+ Dummy tuner driver for common TV standards.
endmenu
@@ -44,5 +44,6 @@ obj-$(CONFIG_MEDIA_TUNER_QM1D1C0042) += qm1d1c0042.o
obj-$(CONFIG_MEDIA_TUNER_QM1D1B0004) += qm1d1b0004.o
obj-$(CONFIG_MEDIA_TUNER_M88RS6000T) += m88rs6000t.o
obj-$(CONFIG_MEDIA_TUNER_TDA18250) += tda18250.o
+obj-$(CONFIG_MEDIA_TUNER_DVB_DUMMY_TUNER) += dvb_dummy_tuner.o
ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
new file mode 100644
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for Dummy Frontend
+ *
+ * Written by Daniel W. S. Almeida <dwlsalmeida@gmail.com>
+ */
+
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <media/dvb_frontend.h>
+#include "dvb_dummy_tuner.h"
+
+struct dvb_dummy_tuner_dev {
+ struct dvb_frontend *fe;
+};
+
+static void dvb_dummy_tuner_release(struct dvb_frontend *fe)
+{
+}
+
+static int dvb_dummy_tuner_init(struct dvb_frontend *fe)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_sleep(struct dvb_frontend *fe)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_suspend(struct dvb_frontend *fe)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_resume(struct dvb_frontend *fe)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_set_params(struct dvb_frontend *fe)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_set_config(struct dvb_frontend *fe, void *priv_cfg)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_get_frequency(struct dvb_frontend *fe,
+ u32 *frequency)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_get_bandwidth(struct dvb_frontend *fe,
+ u32 *bandwidth)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_get_if_frequency(struct dvb_frontend *fe,
+ u32 *frequency)
+{
+ return 0;
+}
+
+static int dvb_dummy_tuner_get_status(struct dvb_frontend *fe, u32 *status)
+{
+ return 0;
+}
+
+static const struct dvb_tuner_ops dvb_dummy_tuner_ops = {
+ .release = dvb_dummy_tuner_release,
+ .init = dvb_dummy_tuner_init,
+ .sleep = dvb_dummy_tuner_sleep,
+ .suspend = dvb_dummy_tuner_suspend,
+ .resume = dvb_dummy_tuner_resume,
+ .set_params = dvb_dummy_tuner_set_params,
+ .set_config = dvb_dummy_tuner_set_config,
+ .get_bandwidth = dvb_dummy_tuner_get_bandwidth,
+ .get_frequency = dvb_dummy_tuner_get_frequency,
+ .get_if_frequency = dvb_dummy_tuner_get_if_frequency,
+ .get_status = dvb_dummy_tuner_get_status,
+};
+
+static const struct i2c_device_id dvb_dummy_tuner_i2c_id_table[] = {
+ {"dvb_dummy_tuner", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, dvb_dummy_tuner_i2c_id_table);
+
+static int dvb_dummy_tuner_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int ret = 0;
+ struct dvb_dummy_tuner_config *config = client->dev.platform_data;
+ struct dvb_frontend *fe = config->fe;
+ struct dvb_dummy_tuner_dev *tuner_dev = NULL;
+
+ tuner_dev = kzalloc(sizeof(*tuner_dev), GFP_KERNEL);
+ if (!tuner_dev) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ i2c_set_clientdata(client, tuner_dev);
+ tuner_dev->fe = config->fe;
+
+ memcpy(&fe->ops.tuner_ops,
+ &dvb_dummy_tuner_ops,
+ sizeof(struct dvb_tuner_ops));
+
+ fe->tuner_priv = client;
+
+ pr_debug("%s: Successfully probed %s\n", __func__, client->name);
+ return ret;
+
+err:
+ pr_err("%s: failed\n", __func__);
+ return ret;
+}
+
+static int dvb_dummy_tuner_i2c_remove(struct i2c_client *client)
+{
+ struct dvb_dummy_tuner_dev *tuner_dev = i2c_get_clientdata(client);
+ struct dvb_frontend *fe = tuner_dev->fe;
+
+ memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
+ fe->tuner_priv = NULL;
+ kfree(tuner_dev);
+
+ return 0;
+}
+
+static struct i2c_driver dvb_dummy_tuner_i2c_driver = {
+ .driver = {
+ .name = "dvb_dummy_tuner",
+ .suppress_bind_attrs = true,
+ },
+ .probe = dvb_dummy_tuner_i2c_probe,
+ .remove = dvb_dummy_tuner_i2c_remove,
+ .id_table = dvb_dummy_tuner_i2c_id_table,
+};
+module_i2c_driver(dvb_dummy_tuner_i2c_driver);
+
+MODULE_DESCRIPTION("DVB Dummy Tuner");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");
new file mode 100644
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * DVB Dummy Tuner
+ *
+ */
+#ifndef __TUNER_DVB_DUMMY_TUNER_H__
+#define __TUNER_DVB_DUMMY_TUNER_H__
+
+#include <media/dvb_frontend.h>
+/**
+ * struct dvb_dummy_tuner_config - configuration parameters for the dummy tuner
+ *
+ * @fe:
+ * frontend returned by driver
+ */
+struct dvb_dummy_tuner_config {
+ struct dvb_frontend *fe;
+};
+
+#endif /*__TUNER_DVB_DUMMY_TUNER_H__ */