* [PATCH 1/3] media: cx231xx: Convert enum into a define
2024-12-02 15:47 [PATCH 0/3] media: Fix warnings with llvm9 Ricardo Ribalda
@ 2024-12-02 15:47 ` Ricardo Ribalda
2024-12-03 8:31 ` Mauro Carvalho Chehab
2024-12-02 15:47 ` [PATCH 2/3] media: atomisp: Use the actual value of the enum instead of the enum Ricardo Ribalda
2024-12-02 15:47 ` [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning Ricardo Ribalda
2 siblings, 1 reply; 12+ messages in thread
From: Ricardo Ribalda @ 2024-12-02 15:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Andy Shevchenko, Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen,
Yunfei Dong, Matthias Brugger, AngeloGioacchino Del Regno
Cc: linux-media, linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek, Ricardo Ribalda
The code is running arithmentics with the enum, which when not done with
care makes the compiler a bit nervous.
Because those enums are only used as defines (no argument or variable
from that enumeration type), convert it into a define and move on.
It is required to build with llvm 9 without these warnings:
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:673:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:680:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:687:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:702:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:709:21: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:718:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:727:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:737:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:749:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
index 5bc44f194d0a..62ffa16bb82c 100644
--- a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
+++ b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
@@ -57,19 +57,17 @@ enum USB_SPEED{
};
#define TS_MASK 0x6
-enum TS_PORT{
- NO_TS_PORT = 0x0, /* 2'b00: Neither port used. PCB not a Hybrid,
+#define NO_TS_PORT 0x0 /* 2'b00: Neither port used. PCB not a Hybrid,
only offers Analog TV or Video */
- TS1_PORT = 0x4, /* 2'b10: TS1 Input (Hybrid mode :
+#define TS1_PORT 0x4 /* 2'b10: TS1 Input (Hybrid mode :
Digital or External Analog/Compressed source) */
- TS1_TS2_PORT = 0x6, /* 2'b11: TS1 & TS2 Inputs
+#define TS1_TS2_PORT 0x6 /* 2'b11: TS1 & TS2 Inputs
(Dual inputs from Digital and/or
External Analog/Compressed sources) */
- TS1_EXT_CLOCK = 0x6, /* 2'b11: TS1 & TS2 as selector
+#define TS1_EXT_CLOCK 0x6 /* 2'b11: TS1 & TS2 as selector
to external clock */
- TS1VIP_TS2_PORT = 0x2 /* 2'b01: TS1 used as 656/VIP Output,
+#define TS1VIP_TS2_PORT 0x2 /* 2'b01: TS1 used as 656/VIP Output,
TS2 Input (from Compressor) */
-};
#define EAVP_MASK 0x8
enum EAV_PRESENT{
@@ -89,10 +87,8 @@ enum AT_MODE{
};
#define PWR_SEL_MASK 0x40
-enum POWE_TYPE{
- SELF_POWER = 0x0, /* 0: self power */
- BUS_POWER = 0x40 /* 1: bus power */
-};
+#define SELF_POWER 0x0 /* 0: self power */
+#define BUS_POWER 0x40 /* 1: bus power */
enum USB_POWE_TYPE{
USB_SELF_POWER = 0,
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] media: cx231xx: Convert enum into a define
2024-12-02 15:47 ` [PATCH 1/3] media: cx231xx: Convert enum into a define Ricardo Ribalda
@ 2024-12-03 8:31 ` Mauro Carvalho Chehab
2025-02-20 13:55 ` Hans Verkuil
0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-03 8:31 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Andy Shevchenko, Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen,
Yunfei Dong, Matthias Brugger, AngeloGioacchino Del Regno,
linux-media, linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek
Em Mon, 02 Dec 2024 15:47:15 +0000
Ricardo Ribalda <ribalda@chromium.org> escreveu:
> The code is running arithmentics with the enum, which when not done with
> care makes the compiler a bit nervous.
>
> Because those enums are only used as defines (no argument or variable
> from that enumeration type), convert it into a define and move on.
>
> It is required to build with llvm 9 without these warnings:
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:673:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:680:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:687:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:702:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:709:21: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:718:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:727:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:737:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:749:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
> index 5bc44f194d0a..62ffa16bb82c 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
> +++ b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
> @@ -57,19 +57,17 @@ enum USB_SPEED{
> };
>
> #define TS_MASK 0x6
> -enum TS_PORT{
> - NO_TS_PORT = 0x0, /* 2'b00: Neither port used. PCB not a Hybrid,
> +#define NO_TS_PORT 0x0 /* 2'b00: Neither port used. PCB not a Hybrid,
> only offers Analog TV or Video */
> - TS1_PORT = 0x4, /* 2'b10: TS1 Input (Hybrid mode :
> +#define TS1_PORT 0x4 /* 2'b10: TS1 Input (Hybrid mode :
> Digital or External Analog/Compressed source) */
> - TS1_TS2_PORT = 0x6, /* 2'b11: TS1 & TS2 Inputs
> +#define TS1_TS2_PORT 0x6 /* 2'b11: TS1 & TS2 Inputs
> (Dual inputs from Digital and/or
> External Analog/Compressed sources) */
> - TS1_EXT_CLOCK = 0x6, /* 2'b11: TS1 & TS2 as selector
> +#define TS1_EXT_CLOCK 0x6 /* 2'b11: TS1 & TS2 as selector
> to external clock */
> - TS1VIP_TS2_PORT = 0x2 /* 2'b01: TS1 used as 656/VIP Output,
> +#define TS1VIP_TS2_PORT 0x2 /* 2'b01: TS1 used as 656/VIP Output,
> TS2 Input (from Compressor) */
> -};
>
> #define EAVP_MASK 0x8
> enum EAV_PRESENT{
> @@ -89,10 +87,8 @@ enum AT_MODE{
> };
>
> #define PWR_SEL_MASK 0x40
> -enum POWE_TYPE{
> - SELF_POWER = 0x0, /* 0: self power */
> - BUS_POWER = 0x40 /* 1: bus power */
> -};
> +#define SELF_POWER 0x0 /* 0: self power */
> +#define BUS_POWER 0x40 /* 1: bus power */
>
> enum USB_POWE_TYPE{
> USB_SELF_POWER = 0,
>
IMO keeping them into enums are a lot better than defines.
Perhaps the right thing would be to join both enums here.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] media: cx231xx: Convert enum into a define
2024-12-03 8:31 ` Mauro Carvalho Chehab
@ 2025-02-20 13:55 ` Hans Verkuil
2025-02-20 14:06 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2025-02-20 13:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Andy Shevchenko, Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen,
Yunfei Dong, Matthias Brugger, AngeloGioacchino Del Regno,
linux-media, linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek
On 12/3/24 09:31, Mauro Carvalho Chehab wrote:
> Em Mon, 02 Dec 2024 15:47:15 +0000
> Ricardo Ribalda <ribalda@chromium.org> escreveu:
>
>> The code is running arithmentics with the enum, which when not done with
>> care makes the compiler a bit nervous.
>>
>> Because those enums are only used as defines (no argument or variable
>> from that enumeration type), convert it into a define and move on.
>>
>> It is required to build with llvm 9 without these warnings:
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:673:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:680:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:687:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:702:17: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:709:21: warning: bitwise operation between different enumeration types ('enum TS_PORT' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:718:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum POWE_TYPE') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:727:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:737:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.c:749:21: warning: bitwise operation between different enumeration types ('enum AVDEC_STATUS' and 'enum TS_PORT') [-Wenum-enum-conversion]
>>
>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>> drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h | 18 +++++++-----------
>> 1 file changed, 7 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
>> index 5bc44f194d0a..62ffa16bb82c 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
>> +++ b/drivers/media/usb/cx231xx/cx231xx-pcb-cfg.h
>> @@ -57,19 +57,17 @@ enum USB_SPEED{
>> };
>>
>> #define TS_MASK 0x6
>> -enum TS_PORT{
>> - NO_TS_PORT = 0x0, /* 2'b00: Neither port used. PCB not a Hybrid,
>> +#define NO_TS_PORT 0x0 /* 2'b00: Neither port used. PCB not a Hybrid,
>> only offers Analog TV or Video */
>> - TS1_PORT = 0x4, /* 2'b10: TS1 Input (Hybrid mode :
>> +#define TS1_PORT 0x4 /* 2'b10: TS1 Input (Hybrid mode :
>> Digital or External Analog/Compressed source) */
>> - TS1_TS2_PORT = 0x6, /* 2'b11: TS1 & TS2 Inputs
>> +#define TS1_TS2_PORT 0x6 /* 2'b11: TS1 & TS2 Inputs
>> (Dual inputs from Digital and/or
>> External Analog/Compressed sources) */
>> - TS1_EXT_CLOCK = 0x6, /* 2'b11: TS1 & TS2 as selector
>> +#define TS1_EXT_CLOCK 0x6 /* 2'b11: TS1 & TS2 as selector
>> to external clock */
>> - TS1VIP_TS2_PORT = 0x2 /* 2'b01: TS1 used as 656/VIP Output,
>> +#define TS1VIP_TS2_PORT 0x2 /* 2'b01: TS1 used as 656/VIP Output,
>> TS2 Input (from Compressor) */
>> -};
>>
>> #define EAVP_MASK 0x8
>> enum EAV_PRESENT{
>> @@ -89,10 +87,8 @@ enum AT_MODE{
>> };
>>
>> #define PWR_SEL_MASK 0x40
>> -enum POWE_TYPE{
>> - SELF_POWER = 0x0, /* 0: self power */
>> - BUS_POWER = 0x40 /* 1: bus power */
>> -};
>> +#define SELF_POWER 0x0 /* 0: self power */
>> +#define BUS_POWER 0x40 /* 1: bus power */
>>
>> enum USB_POWE_TYPE{
>> USB_SELF_POWER = 0,
>>
>
> IMO keeping them into enums are a lot better than defines.
>
> Perhaps the right thing would be to join both enums here.
ORing enums is really not a good idea: you would normally never do that, and the
compiler warning is IMHO appropriate.
I think this is a good change and I plan to take this.
Regards,
Hans
>
>
> Thanks,
> Mauro
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] media: cx231xx: Convert enum into a define
2025-02-20 13:55 ` Hans Verkuil
@ 2025-02-20 14:06 ` Andy Shevchenko
2025-02-20 15:12 ` Hans Verkuil
0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2025-02-20 14:06 UTC (permalink / raw)
To: Hans Verkuil
Cc: Mauro Carvalho Chehab, Ricardo Ribalda, Mauro Carvalho Chehab,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Hans de Goede, Sakari Ailus, Greg Kroah-Hartman, Tiffany Lin,
Andrew-CT Chen, Yunfei Dong, Matthias Brugger,
AngeloGioacchino Del Regno, linux-media, linux-kernel, llvm,
linux-staging, linux-arm-kernel, linux-mediatek
On Thu, Feb 20, 2025 at 02:55:42PM +0100, Hans Verkuil wrote:
> On 12/3/24 09:31, Mauro Carvalho Chehab wrote:
...
> ORing enums is really not a good idea: you would normally never do that, and
I think you missed a keyword "different", so "ORing different enums ..."
which I totally agree on, but the same enum values are fine.
> the compiler warning is IMHO appropriate.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] media: cx231xx: Convert enum into a define
2025-02-20 14:06 ` Andy Shevchenko
@ 2025-02-20 15:12 ` Hans Verkuil
2025-02-20 15:25 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2025-02-20 15:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mauro Carvalho Chehab, Ricardo Ribalda, Mauro Carvalho Chehab,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Hans de Goede, Sakari Ailus, Greg Kroah-Hartman, Tiffany Lin,
Andrew-CT Chen, Yunfei Dong, Matthias Brugger,
AngeloGioacchino Del Regno, linux-media, linux-kernel, llvm,
linux-staging, linux-arm-kernel, linux-mediatek
On 2/20/25 15:06, Andy Shevchenko wrote:
> On Thu, Feb 20, 2025 at 02:55:42PM +0100, Hans Verkuil wrote:
>> On 12/3/24 09:31, Mauro Carvalho Chehab wrote:
>
> ...
>
>> ORing enums is really not a good idea: you would normally never do that, and
>
> I think you missed a keyword "different", so "ORing different enums ..."
> which I totally agree on, but the same enum values are fine.
While the compiler might be happy with that, I think ORing enums regardless
is weird. It's not what enums are for.
Regards,
Hans
>
>> the compiler warning is IMHO appropriate.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] media: cx231xx: Convert enum into a define
2025-02-20 15:12 ` Hans Verkuil
@ 2025-02-20 15:25 ` Andy Shevchenko
0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2025-02-20 15:25 UTC (permalink / raw)
To: Hans Verkuil
Cc: Mauro Carvalho Chehab, Ricardo Ribalda, Mauro Carvalho Chehab,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Hans de Goede, Sakari Ailus, Greg Kroah-Hartman, Tiffany Lin,
Andrew-CT Chen, Yunfei Dong, Matthias Brugger,
AngeloGioacchino Del Regno, linux-media, linux-kernel, llvm,
linux-staging, linux-arm-kernel, linux-mediatek
On Thu, Feb 20, 2025 at 04:12:38PM +0100, Hans Verkuil wrote:
> On 2/20/25 15:06, Andy Shevchenko wrote:
> > On Thu, Feb 20, 2025 at 02:55:42PM +0100, Hans Verkuil wrote:
> >> On 12/3/24 09:31, Mauro Carvalho Chehab wrote:
...
> >> ORing enums is really not a good idea: you would normally never do that, and
> >
> > I think you missed a keyword "different", so "ORing different enums ..."
> > which I totally agree on, but the same enum values are fine.
>
> While the compiler might be happy with that, I think ORing enums regardless
> is weird. It's not what enums are for.
I disagree. It's totally normal to have "mixed" enums where we have ranges of
sequential values mixed with bit flags. Cross-enum bit operations (and what
this patch is about) should be prohibited, indeed.
> >> the compiler warning is IMHO appropriate.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] media: atomisp: Use the actual value of the enum instead of the enum
2024-12-02 15:47 [PATCH 0/3] media: Fix warnings with llvm9 Ricardo Ribalda
2024-12-02 15:47 ` [PATCH 1/3] media: cx231xx: Convert enum into a define Ricardo Ribalda
@ 2024-12-02 15:47 ` Ricardo Ribalda
2024-12-02 16:14 ` Andy Shevchenko
2024-12-02 15:47 ` [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning Ricardo Ribalda
2 siblings, 1 reply; 12+ messages in thread
From: Ricardo Ribalda @ 2024-12-02 15:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Andy Shevchenko, Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen,
Yunfei Dong, Matthias Brugger, AngeloGioacchino Del Regno
Cc: linux-media, linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek, Ricardo Ribalda
hrt_isp_css_irq_sw_pin_0 has a different enum type than
irq_sw_channel_id_t.
Replace it with the actual value of hrt_isp_css_irq_sw_pin_0 to avoid
arithmetic operations between different enum types (and make the
compiler happy).
It is required to build with llvm 9 without these warnings:
drivers/staging/media/atomisp/pci/sh_css_hrt.c:68:19: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
drivers/staging/media/atomisp/pci/sh_css.c:1233:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
drivers/staging/media/atomisp/pci/sh_css.c:1237:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
ro
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/staging/media/atomisp/pci/hive_isp_css_common/irq_global.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/irq_global.h b/drivers/staging/media/atomisp/pci/hive_isp_css_common/irq_global.h
index 2c47e7820bd7..69e68ecd6bc3 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/irq_global.h
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/irq_global.h
@@ -18,7 +18,7 @@
#endif
/* The IRQ is not mapped uniformly on its related interfaces */
-#define IRQ_SW_CHANNEL_OFFSET hrt_isp_css_irq_sw_pin_0
+#define IRQ_SW_CHANNEL_OFFSET HIVE_GP_DEV_IRQ_SW_PIN_0_BIT_ID
typedef enum {
IRQ_SW_CHANNEL0_ID = hrt_isp_css_irq_sw_pin_0 - IRQ_SW_CHANNEL_OFFSET,
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] media: atomisp: Use the actual value of the enum instead of the enum
2024-12-02 15:47 ` [PATCH 2/3] media: atomisp: Use the actual value of the enum instead of the enum Ricardo Ribalda
@ 2024-12-02 16:14 ` Andy Shevchenko
0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2024-12-02 16:14 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Matthias Brugger, AngeloGioacchino Del Regno, linux-media,
linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek
On Mon, Dec 02, 2024 at 03:47:16PM +0000, Ricardo Ribalda wrote:
> hrt_isp_css_irq_sw_pin_0 has a different enum type than
> irq_sw_channel_id_t.
>
> Replace it with the actual value of hrt_isp_css_irq_sw_pin_0 to avoid
> arithmetic operations between different enum types (and make the
> compiler happy).
>
> It is required to build with llvm 9 without these warnings:
9?! Maybe 19?
> drivers/staging/media/atomisp/pci/sh_css_hrt.c:68:19: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
> drivers/staging/media/atomisp/pci/sh_css.c:1233:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
> drivers/staging/media/atomisp/pci/sh_css.c:1237:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
> ro
You can make the above lines shorter by dropping the obvious an duplicated (in
other places) part, i.e. drivers/staging/media/atomisp/. I usually replace
this with "...".
.../pci/sh_css_hrt.c:68:19: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
.../pci/sh_css.c:1233:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
.../pci/sh_css.c:1237:40: warning: arithmetic between different enumeration types ('irq_sw_channel_id_t' and 'enum hrt_isp_css_irq') [-Wenum-enum-conversion]
Otherwise LGTM,
Reviewed-by: Andy Shevchenko <andy@kernel.org>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning
2024-12-02 15:47 [PATCH 0/3] media: Fix warnings with llvm9 Ricardo Ribalda
2024-12-02 15:47 ` [PATCH 1/3] media: cx231xx: Convert enum into a define Ricardo Ribalda
2024-12-02 15:47 ` [PATCH 2/3] media: atomisp: Use the actual value of the enum instead of the enum Ricardo Ribalda
@ 2024-12-02 15:47 ` Ricardo Ribalda
2024-12-02 16:24 ` Nathan Chancellor
2 siblings, 1 reply; 12+ messages in thread
From: Ricardo Ribalda @ 2024-12-02 15:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Nathan Chancellor, Nick Desaulniers,
Bill Wendling, Justin Stitt, Hans de Goede, Sakari Ailus,
Andy Shevchenko, Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen,
Yunfei Dong, Matthias Brugger, AngeloGioacchino Del Regno
Cc: linux-media, linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek, Ricardo Ribalda
llvm identifies that the SCP_IPI_VENC_H264 and IPI_VENC_H264 are from
the same enum type, but their are part of the same ternary operator.
vpu_inst.id is of type int, so we can just rewrite a bit the code and
avoid the following llvm9 warning:
drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c:597:29: warning: conditional expression between different enumeration types ('enum scp_ipi_id' and 'enum ipi_id') [-Wenum-compare-conditional]
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
index f8145998fcaf..4786062e879a 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
@@ -584,7 +584,6 @@ static void h264_encode_filler(struct venc_h264_inst *inst, void *buf,
static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
{
- const bool is_ext = MTK_ENC_CTX_IS_EXT(ctx);
int ret = 0;
struct venc_h264_inst *inst;
@@ -594,7 +593,10 @@ static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
inst->ctx = ctx;
inst->vpu_inst.ctx = ctx;
- inst->vpu_inst.id = is_ext ? SCP_IPI_VENC_H264 : IPI_VENC_H264;
+ if (MTK_ENC_CTX_IS_EXT(ctx))
+ inst->vpu_inst.id = SCP_IPI_VENC_H264;
+ else
+ inst->vpu_inst.id = IPI_VENC_H264;
inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_SYS);
ret = vpu_enc_init(&inst->vpu_inst);
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning
2024-12-02 15:47 ` [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning Ricardo Ribalda
@ 2024-12-02 16:24 ` Nathan Chancellor
2024-12-02 17:26 ` Ricardo Ribalda
0 siblings, 1 reply; 12+ messages in thread
From: Nathan Chancellor @ 2024-12-02 16:24 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Nick Desaulniers, Bill Wendling,
Justin Stitt, Hans de Goede, Sakari Ailus, Andy Shevchenko,
Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Matthias Brugger, AngeloGioacchino Del Regno, linux-media,
linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek
Mon, Dec 02, 2024 at 03:47:17PM +0000, Ricardo Ribalda wrote:
> llvm identifies that the SCP_IPI_VENC_H264 and IPI_VENC_H264 are from
> the same enum type, but their are part of the same ternary operator.
>
> vpu_inst.id is of type int, so we can just rewrite a bit the code and
> avoid the following llvm9 warning:
LLVM 19, not LLVM 9, as the minimum version for building the kernel is
LLVM 13.
> drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c:597:29: warning: conditional expression between different enumeration types ('enum scp_ipi_id' and 'enum ipi_id') [-Wenum-compare-conditional]
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
FYI, Arnd basically sent the same patch October 18 but I guess it has
not been picked up?
https://lore.kernel.org/20241018152127.3958436-1-arnd@kernel.org/
Hopefully the new media committers model will help patches like that get
picked up in a more timely manner.
> ---
> drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> index f8145998fcaf..4786062e879a 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> @@ -584,7 +584,6 @@ static void h264_encode_filler(struct venc_h264_inst *inst, void *buf,
>
> static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
> {
> - const bool is_ext = MTK_ENC_CTX_IS_EXT(ctx);
> int ret = 0;
> struct venc_h264_inst *inst;
>
> @@ -594,7 +593,10 @@ static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
>
> inst->ctx = ctx;
> inst->vpu_inst.ctx = ctx;
> - inst->vpu_inst.id = is_ext ? SCP_IPI_VENC_H264 : IPI_VENC_H264;
> + if (MTK_ENC_CTX_IS_EXT(ctx))
> + inst->vpu_inst.id = SCP_IPI_VENC_H264;
> + else
> + inst->vpu_inst.id = IPI_VENC_H264;
> inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_SYS);
>
> ret = vpu_enc_init(&inst->vpu_inst);
>
> --
> 2.47.0.338.g60cca15819-goog
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] media: mediatek: vcodec: Workaround a compiler warning
2024-12-02 16:24 ` Nathan Chancellor
@ 2024-12-02 17:26 ` Ricardo Ribalda
0 siblings, 0 replies; 12+ messages in thread
From: Ricardo Ribalda @ 2024-12-02 17:26 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Mauro Carvalho Chehab, Nick Desaulniers, Bill Wendling,
Justin Stitt, Hans de Goede, Sakari Ailus, Andy Shevchenko,
Greg Kroah-Hartman, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Matthias Brugger, AngeloGioacchino Del Regno, linux-media,
linux-kernel, llvm, linux-staging, linux-arm-kernel,
linux-mediatek
Hi
On Mon, 2 Dec 2024 at 17:25, Nathan Chancellor <nathan@kernel.org> wrote:
>
> Mon, Dec 02, 2024 at 03:47:17PM +0000, Ricardo Ribalda wrote:
> > llvm identifies that the SCP_IPI_VENC_H264 and IPI_VENC_H264 are from
> > the same enum type, but their are part of the same ternary operator.
> >
> > vpu_inst.id is of type int, so we can just rewrite a bit the code and
> > avoid the following llvm9 warning:
>
> LLVM 19, not LLVM 9, as the minimum version for building the kernel is
> LLVM 13.
>
> > drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c:597:29: warning: conditional expression between different enumeration types ('enum scp_ipi_id' and 'enum ipi_id') [-Wenum-compare-conditional]
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> FYI, Arnd basically sent the same patch October 18 but I guess it has
> not been picked up?
>
> https://lore.kernel.org/20241018152127.3958436-1-arnd@kernel.org/
>
> Hopefully the new media committers model will help patches like that get
> picked up in a more timely manner.
We can take Arnd's patch.
Yeah, this is the kind of patches that the multi committers can really
help with :)
>
> > ---
> > drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> > index f8145998fcaf..4786062e879a 100644
> > --- a/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> > +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc/venc_h264_if.c
> > @@ -584,7 +584,6 @@ static void h264_encode_filler(struct venc_h264_inst *inst, void *buf,
> >
> > static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
> > {
> > - const bool is_ext = MTK_ENC_CTX_IS_EXT(ctx);
> > int ret = 0;
> > struct venc_h264_inst *inst;
> >
> > @@ -594,7 +593,10 @@ static int h264_enc_init(struct mtk_vcodec_enc_ctx *ctx)
> >
> > inst->ctx = ctx;
> > inst->vpu_inst.ctx = ctx;
> > - inst->vpu_inst.id = is_ext ? SCP_IPI_VENC_H264 : IPI_VENC_H264;
> > + if (MTK_ENC_CTX_IS_EXT(ctx))
> > + inst->vpu_inst.id = SCP_IPI_VENC_H264;
> > + else
> > + inst->vpu_inst.id = IPI_VENC_H264;
> > inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_SYS);
> >
> > ret = vpu_enc_init(&inst->vpu_inst);
> >
> > --
> > 2.47.0.338.g60cca15819-goog
> >
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 12+ messages in thread