* [PATCH] [media] cec: add missing inline stubs
@ 2016-07-01 11:19 Arnd Bergmann
2016-07-01 14:35 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-07-01 11:19 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab
Cc: Arnd Bergmann, linux-media, linux-kernel
The linux/cec.h header file contains empty inline definitions for
a subset of the API for the case in which CEC is not enabled,
however we have driver that call other functions as well:
drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_tx_raw_status':
drivers/media/i2c/adv7604.c:1956:3: error: implicit declaration of function 'cec_transmit_done' [-Werror=implicit-function-declaration]
drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_isr':
drivers/media/i2c/adv7604.c:2012:4: error: implicit declaration of function 'cec_received_msg' [-Werror=implicit-function-declaration]
drivers/media/i2c/adv7604.c: In function 'adv76xx_probe':
drivers/media/i2c/adv7604.c:3482:20: error: implicit declaration of function 'cec_allocate_adapter' [-Werror=implicit-function-declaration]
This adds stubs for the remaining interfaces as well.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/media/cec.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/include/media/cec.h b/include/media/cec.h
index 9a791c08a789..c462f9b44074 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -208,6 +208,12 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
#else
+static inline struct cec_adapter *cec_allocate_adapter(
+ const struct cec_adap_ops *ops, void *priv, const char *name,
+ u32 caps, u8 available_las, struct device *parent)
+{
+ return NULL;
+}
static inline int cec_register_adapter(struct cec_adapter *adap)
{
@@ -227,6 +233,25 @@ static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr,
{
}
+static inline int cec_transmit_msg(struct cec_adapter *adap,
+ struct cec_msg *msg, bool block)
+{
+ return 0;
+}
+
+/* Called by the adapter */
+static inline void cec_transmit_done(struct cec_adapter *adap, u8 status,
+ u8 arb_lost_cnt, u8 nack_cnt,
+ u8 low_drive_cnt, u8 error_cnt)
+{
+}
+
+static inline void cec_received_msg(struct cec_adapter *adap,
+ struct cec_msg *msg)
+{
+}
+
+
#endif
#endif /* _MEDIA_CEC_H */
--
2.9.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] cec: add missing inline stubs
2016-07-01 11:19 [PATCH] [media] cec: add missing inline stubs Arnd Bergmann
@ 2016-07-01 14:35 ` Hans Verkuil
2016-07-01 15:13 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2016-07-01 14:35 UTC (permalink / raw)
To: Arnd Bergmann, Hans Verkuil, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel
On 07/01/2016 01:19 PM, Arnd Bergmann wrote:
> The linux/cec.h header file contains empty inline definitions for
> a subset of the API for the case in which CEC is not enabled,
> however we have driver that call other functions as well:
>
> drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_tx_raw_status':
> drivers/media/i2c/adv7604.c:1956:3: error: implicit declaration of function 'cec_transmit_done' [-Werror=implicit-function-declaration]
> drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_isr':
> drivers/media/i2c/adv7604.c:2012:4: error: implicit declaration of function 'cec_received_msg' [-Werror=implicit-function-declaration]
> drivers/media/i2c/adv7604.c: In function 'adv76xx_probe':
> drivers/media/i2c/adv7604.c:3482:20: error: implicit declaration of function 'cec_allocate_adapter' [-Werror=implicit-function-declaration]
I don't understand this. These calls are under #if IS_ENABLED(CONFIG_VIDEO_ADV7842_CEC),
and that should be 0 if MEDIA_CEC is not selected.
Am I missing some weird config combination?
Regards,
Hans
>
> This adds stubs for the remaining interfaces as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/media/cec.h | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/include/media/cec.h b/include/media/cec.h
> index 9a791c08a789..c462f9b44074 100644
> --- a/include/media/cec.h
> +++ b/include/media/cec.h
> @@ -208,6 +208,12 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
> void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
>
> #else
> +static inline struct cec_adapter *cec_allocate_adapter(
> + const struct cec_adap_ops *ops, void *priv, const char *name,
> + u32 caps, u8 available_las, struct device *parent)
> +{
> + return NULL;
> +}
>
> static inline int cec_register_adapter(struct cec_adapter *adap)
> {
> @@ -227,6 +233,25 @@ static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr,
> {
> }
>
> +static inline int cec_transmit_msg(struct cec_adapter *adap,
> + struct cec_msg *msg, bool block)
> +{
> + return 0;
> +}
> +
> +/* Called by the adapter */
> +static inline void cec_transmit_done(struct cec_adapter *adap, u8 status,
> + u8 arb_lost_cnt, u8 nack_cnt,
> + u8 low_drive_cnt, u8 error_cnt)
> +{
> +}
> +
> +static inline void cec_received_msg(struct cec_adapter *adap,
> + struct cec_msg *msg)
> +{
> +}
> +
> +
> #endif
>
> #endif /* _MEDIA_CEC_H */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] cec: add missing inline stubs
2016-07-01 14:35 ` Hans Verkuil
@ 2016-07-01 15:13 ` Arnd Bergmann
2016-07-01 15:22 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-07-01 15:13 UTC (permalink / raw)
To: Hans Verkuil
Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-kernel
On Friday, July 1, 2016 4:35:09 PM CEST Hans Verkuil wrote:
> On 07/01/2016 01:19 PM, Arnd Bergmann wrote:
> > The linux/cec.h header file contains empty inline definitions for
> > a subset of the API for the case in which CEC is not enabled,
> > however we have driver that call other functions as well:
> >
> > drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_tx_raw_status':
> > drivers/media/i2c/adv7604.c:1956:3: error: implicit declaration of function 'cec_transmit_done' [-Werror=implicit-function-declaration]
> > drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_isr':
> > drivers/media/i2c/adv7604.c:2012:4: error: implicit declaration of function 'cec_received_msg' [-Werror=implicit-function-declaration]
> > drivers/media/i2c/adv7604.c: In function 'adv76xx_probe':
> > drivers/media/i2c/adv7604.c:3482:20: error: implicit declaration of function 'cec_allocate_adapter' [-Werror=implicit-function-declaration]
>
> I don't understand this. These calls are under #if IS_ENABLED(CONFIG_VIDEO_ADV7842_CEC),
> and that should be 0 if MEDIA_CEC is not selected.
>
> Am I missing some weird config combination?
This must have been a build error I ran into before your patch, when I
had this one applied locally instead:
diff --git a/include/media/cec.h b/include/media/cec.h
index c462f9b44074..564a6a06bed7 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -187,7 +187,7 @@ static inline bool cec_is_sink(const struct cec_adapter *adap)
return adap->phys_addr == 0;
}
-#if IS_ENABLED(CONFIG_MEDIA_CEC)
+#if IS_REACHABLE(CONFIG_MEDIA_CEC)
struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
void *priv, const char *name, u32 caps, u8 available_las,
struct device *parent);
because that was hiding the declarations when the code could not
reach it. With your newer patch that is not possible any more.
I also wasn't aware that each of these already had their own Kconfig
symbols. Could we just do this instead of your patch then:
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index ce9006e10a30..73e047220905 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -222,6 +222,7 @@ config VIDEO_ADV7604
config VIDEO_ADV7604_CEC
bool "Enable Analog Devices ADV7604 CEC support"
depends on VIDEO_ADV7604 && MEDIA_CEC
+ depends on VIDEO_ADV7604=m || MEDIA_CEC=y
---help---
When selected the adv7604 will support the optional
HDMI CEC feature.
@@ -243,6 +244,7 @@ config VIDEO_ADV7842
config VIDEO_ADV7842_CEC
bool "Enable Analog Devices ADV7842 CEC support"
depends on VIDEO_ADV7842 && MEDIA_CEC
+ depends on VIDEO_ADV7842=m || MEDIA_CEC=y
---help---
When selected the adv7842 will support the optional
HDMI CEC feature.
@@ -475,6 +477,7 @@ config VIDEO_ADV7511
config VIDEO_ADV7511_CEC
bool "Enable Analog Devices ADV7511 CEC support"
depends on VIDEO_ADV7511 && MEDIA_CEC
+ depends on VIDEO_ADV7511=m || MEDIA_CEC=y
---help---
When selected the adv7511 will support the optional
HDMI CEC feature.
diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
index 8e6918c5c87c..8e31146d079a 100644
--- a/drivers/media/platform/vivid/Kconfig
+++ b/drivers/media/platform/vivid/Kconfig
@@ -26,6 +26,7 @@ config VIDEO_VIVID
config VIDEO_VIVID_CEC
bool "Enable CEC emulation support"
depends on VIDEO_VIVID && MEDIA_CEC
+ depends on VIDEO_VIVID=m || MEDIA_CEC=y
---help---
When selected the vivid module will emulate the optional
HDMI CEC feature.
which is still not overly nice, but it manages to avoid the
IS_REACHABLE() check and it lets MEDIA_CEC be a module.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] cec: add missing inline stubs
2016-07-01 15:13 ` Arnd Bergmann
@ 2016-07-01 15:22 ` Hans Verkuil
2016-07-01 15:37 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2016-07-01 15:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-kernel
On 07/01/2016 05:13 PM, Arnd Bergmann wrote:
> On Friday, July 1, 2016 4:35:09 PM CEST Hans Verkuil wrote:
>> On 07/01/2016 01:19 PM, Arnd Bergmann wrote:
>>> The linux/cec.h header file contains empty inline definitions for
>>> a subset of the API for the case in which CEC is not enabled,
>>> however we have driver that call other functions as well:
>>>
>>> drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_tx_raw_status':
>>> drivers/media/i2c/adv7604.c:1956:3: error: implicit declaration of function 'cec_transmit_done' [-Werror=implicit-function-declaration]
>>> drivers/media/i2c/adv7604.c: In function 'adv76xx_cec_isr':
>>> drivers/media/i2c/adv7604.c:2012:4: error: implicit declaration of function 'cec_received_msg' [-Werror=implicit-function-declaration]
>>> drivers/media/i2c/adv7604.c: In function 'adv76xx_probe':
>>> drivers/media/i2c/adv7604.c:3482:20: error: implicit declaration of function 'cec_allocate_adapter' [-Werror=implicit-function-declaration]
>>
>> I don't understand this. These calls are under #if IS_ENABLED(CONFIG_VIDEO_ADV7842_CEC),
>> and that should be 0 if MEDIA_CEC is not selected.
>>
>> Am I missing some weird config combination?
>
> This must have been a build error I ran into before your patch, when I
> had this one applied locally instead:
>
> diff --git a/include/media/cec.h b/include/media/cec.h
> index c462f9b44074..564a6a06bed7 100644
> --- a/include/media/cec.h
> +++ b/include/media/cec.h
> @@ -187,7 +187,7 @@ static inline bool cec_is_sink(const struct cec_adapter *adap)
> return adap->phys_addr == 0;
> }
>
> -#if IS_ENABLED(CONFIG_MEDIA_CEC)
> +#if IS_REACHABLE(CONFIG_MEDIA_CEC)
> struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
> void *priv, const char *name, u32 caps, u8 available_las,
> struct device *parent);
>
> because that was hiding the declarations when the code could not
> reach it. With your newer patch that is not possible any more.
>
> I also wasn't aware that each of these already had their own Kconfig
> symbols. Could we just do this instead of your patch then:
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index ce9006e10a30..73e047220905 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -222,6 +222,7 @@ config VIDEO_ADV7604
> config VIDEO_ADV7604_CEC
> bool "Enable Analog Devices ADV7604 CEC support"
> depends on VIDEO_ADV7604 && MEDIA_CEC
> + depends on VIDEO_ADV7604=m || MEDIA_CEC=y
> ---help---
> When selected the adv7604 will support the optional
> HDMI CEC feature.
> @@ -243,6 +244,7 @@ config VIDEO_ADV7842
> config VIDEO_ADV7842_CEC
> bool "Enable Analog Devices ADV7842 CEC support"
> depends on VIDEO_ADV7842 && MEDIA_CEC
> + depends on VIDEO_ADV7842=m || MEDIA_CEC=y
> ---help---
> When selected the adv7842 will support the optional
> HDMI CEC feature.
> @@ -475,6 +477,7 @@ config VIDEO_ADV7511
> config VIDEO_ADV7511_CEC
> bool "Enable Analog Devices ADV7511 CEC support"
> depends on VIDEO_ADV7511 && MEDIA_CEC
> + depends on VIDEO_ADV7511=m || MEDIA_CEC=y
> ---help---
> When selected the adv7511 will support the optional
> HDMI CEC feature.
> diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
> index 8e6918c5c87c..8e31146d079a 100644
> --- a/drivers/media/platform/vivid/Kconfig
> +++ b/drivers/media/platform/vivid/Kconfig
> @@ -26,6 +26,7 @@ config VIDEO_VIVID
> config VIDEO_VIVID_CEC
> bool "Enable CEC emulation support"
> depends on VIDEO_VIVID && MEDIA_CEC
> + depends on VIDEO_VIVID=m || MEDIA_CEC=y
> ---help---
> When selected the vivid module will emulate the optional
> HDMI CEC feature.
>
> which is still not overly nice, but it manages to avoid the
> IS_REACHABLE() check and it lets MEDIA_CEC be a module.
The only IS_REACHABLE is for the RC_CORE check, and that should remain.
With my patch MEDIA_CEC can remain a module provided MEDIA_SUPPORT is also
a module. All drivers depending on MEDIA_CEC also depend on MEDIA_SUPPORT,
so that works.
As I mentioned in a previous post, this will change in the future but while
it is in staging I prefer to keep it like this.
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] cec: add missing inline stubs
2016-07-01 15:22 ` Hans Verkuil
@ 2016-07-01 15:37 ` Arnd Bergmann
2016-07-01 17:30 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-07-01 15:37 UTC (permalink / raw)
To: Hans Verkuil
Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-kernel
On Friday, July 1, 2016 5:22:32 PM CEST Hans Verkuil wrote:
> > diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
> > index 8e6918c5c87c..8e31146d079a 100644
> > --- a/drivers/media/platform/vivid/Kconfig
> > +++ b/drivers/media/platform/vivid/Kconfig
> > @@ -26,6 +26,7 @@ config VIDEO_VIVID
> > config VIDEO_VIVID_CEC
> > bool "Enable CEC emulation support"
> > depends on VIDEO_VIVID && MEDIA_CEC
> > + depends on VIDEO_VIVID=m || MEDIA_CEC=y
> > ---help---
> > When selected the vivid module will emulate the optional
> > HDMI CEC feature.
> >
> > which is still not overly nice, but it manages to avoid the
> > IS_REACHABLE() check and it lets MEDIA_CEC be a module.
>
> The only IS_REACHABLE is for the RC_CORE check, and that should remain.
I believe that is already taken care of by my earlier "[media] cec: add
RC_CORE dependency" patch, https://patchwork.linuxtv.org/patch/34892/
which seems to handle the dependency more gracefully (preventing nonsense
configurations rather than just not using RC_CORE).
> With my patch MEDIA_CEC can remain a module provided MEDIA_SUPPORT is also
> a module. All drivers depending on MEDIA_CEC also depend on MEDIA_SUPPORT,
> so that works.
To clarify, the problem with the option above is that VIDEO_VIVID_CEC
is a 'bool' option, and Kconfig lets that be turned on if both
VIDEO_VIVID and MEDIA_CEC are enabled, including the case where MEDIA_CEC
is a module and VIDEO_VIVID is not.
Your patch avoids that problem by making MEDIA_CEC a 'bool', my patch
above is an alternative by ensuring that VIDEO_VIVID_CEC cannot be
enabled if MEDIA_CEC is a module and VIDEO_VIVID is not.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] cec: add missing inline stubs
2016-07-01 15:37 ` Arnd Bergmann
@ 2016-07-01 17:30 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2016-07-01 17:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media, linux-kernel
On 07/01/2016 05:37 PM, Arnd Bergmann wrote:
> On Friday, July 1, 2016 5:22:32 PM CEST Hans Verkuil wrote:
>>> diff --git a/drivers/media/platform/vivid/Kconfig b/drivers/media/platform/vivid/Kconfig
>>> index 8e6918c5c87c..8e31146d079a 100644
>>> --- a/drivers/media/platform/vivid/Kconfig
>>> +++ b/drivers/media/platform/vivid/Kconfig
>>> @@ -26,6 +26,7 @@ config VIDEO_VIVID
>>> config VIDEO_VIVID_CEC
>>> bool "Enable CEC emulation support"
>>> depends on VIDEO_VIVID && MEDIA_CEC
>>> + depends on VIDEO_VIVID=m || MEDIA_CEC=y
>>> ---help---
>>> When selected the vivid module will emulate the optional
>>> HDMI CEC feature.
>>>
>>> which is still not overly nice, but it manages to avoid the
>>> IS_REACHABLE() check and it lets MEDIA_CEC be a module.
>>
>> The only IS_REACHABLE is for the RC_CORE check, and that should remain.
>
> I believe that is already taken care of by my earlier "[media] cec: add
> RC_CORE dependency" patch, https://patchwork.linuxtv.org/patch/34892/
> which seems to handle the dependency more gracefully (preventing nonsense
> configurations rather than just not using RC_CORE).
>
>> With my patch MEDIA_CEC can remain a module provided MEDIA_SUPPORT is also
>> a module. All drivers depending on MEDIA_CEC also depend on MEDIA_SUPPORT,
>> so that works.
>
> To clarify, the problem with the option above is that VIDEO_VIVID_CEC
> is a 'bool' option, and Kconfig lets that be turned on if both
> VIDEO_VIVID and MEDIA_CEC are enabled, including the case where MEDIA_CEC
> is a module and VIDEO_VIVID is not.
>
> Your patch avoids that problem by making MEDIA_CEC a 'bool', my patch
> above is an alternative by ensuring that VIDEO_VIVID_CEC cannot be
> enabled if MEDIA_CEC is a module and VIDEO_VIVID is not.
The problem I have with both your suggestions is that options disappear just
because one dependency is a module. Very unexpected and hard to figure out.
I am not enthusiastic about the IS_REACHABLE(RC_CORE) solution either, but it
will do for now while it is in staging. I still think it is better then making
MEDIA_CEC depend on whether RC_CORE is a module or not. The RC support is an
optional extension and CEC still works fine without it.
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-01 17:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01 11:19 [PATCH] [media] cec: add missing inline stubs Arnd Bergmann
2016-07-01 14:35 ` Hans Verkuil
2016-07-01 15:13 ` Arnd Bergmann
2016-07-01 15:22 ` Hans Verkuil
2016-07-01 15:37 ` Arnd Bergmann
2016-07-01 17:30 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®