Re: [REVIEW PATCH 02/16] e4000: implement controls via v4l2 control framework

[Mauro Carvalho Chehab <m.chehab@samsung.com>]

Em Thu, 27 Feb 2014 02:30:11 +0200
Antti Palosaari <crope@iki.fi> escreveu:

> Implement gain and bandwidth controls using v4l2 control framework.
> Pointer to control handler is provided by exported symbol.
> 
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/tuners/e4000.c      | 210 +++++++++++++++++++++++++++++++++++++-
>  drivers/media/tuners/e4000.h      |  14 +++
>  drivers/media/tuners/e4000_priv.h |  75 ++++++++++++++
>  3 files changed, 298 insertions(+), 1 deletion(-)

...
> diff --git a/drivers/media/tuners/e4000.h b/drivers/media/tuners/e4000.h
> index e74b8b2..989f2ea 100644
> --- a/drivers/media/tuners/e4000.h
> +++ b/drivers/media/tuners/e4000.h
> @@ -40,4 +40,18 @@ struct e4000_config {
>  	u32 clock;
>  };
>  
> +#if IS_ENABLED(CONFIG_MEDIA_TUNER_E4000)
> +extern struct v4l2_ctrl_handler *e4000_get_ctrl_handler(
> +		struct dvb_frontend *fe
> +);
> +#else
> +static inline struct v4l2_ctrl_handler *e4000_get_ctrl_handler(
> +		struct dvb_frontend *fe
> +)
> +{
> +	pr_warn("%s: driver disabled by Kconfig\n", __func__);
> +	return NULL;
> +}
> +#endif
> +
>  #endif

There are two things to be noticed here:

1) Please don't add any EXPORT_SYMBOL() on a pure I2C module. You
should, instead, use the subdev calls, in order to callback a
function provided by the module;

2) As you're now using request_module(), you don't need to use
#if IS_ENABLED() anymore. It is up to the module to register
itself as a V4L2 subdevice. The caller module should use the
subdevice interface to run the callbacks.

If you don't to that, you'll have several issues with the
building system.

Regards,
Mauro