[PATCH 00/10] Address some clang W=1 warnings

[PATCH 00/10] Address some clang W=1 warnings

[Mauro Carvalho Chehab]

Clang is more pedantic than gcc with W=1. This is currently causing
builder.linuxtv.org errors due to CONFIG_WERROR.

Address some of those.

Mauro Carvalho Chehab (10):
  media: cx25821: drop duplicated i2c_slave_did_ack()
  media: ivtv: drop an unused macro
  media: cx18: drop an unused macro
  media: stb6100: mark a currently unused function as such
  media: mc: mark a debug function with __maybe_unused
  media: dvb-core: dvb_frontend: address some clang warnings
  media: cx25840: drop some unused inline functions
  media: marvell-ccic: drop to_cam() unused function
  media: omap3isp: mark isp_isr_dbg as __maybe_unused
  media: omap3isp: avoid warnings at IS_OUT_OF_BOUNDS()

 drivers/media/dvb-core/dvb_frontend.c         | 13 +++++-------
 drivers/media/dvb-frontends/stb6100.c         |  2 +-
 drivers/media/i2c/cx25840/cx25840-ir.c        | 20 -------------------
 drivers/media/mc/mc-entity.c                  |  2 +-
 drivers/media/pci/cx18/cx18-alsa-main.c       |  6 ------
 drivers/media/pci/cx25821/cx25821-core.c      |  7 -------
 drivers/media/pci/ivtv/ivtv-alsa-main.c       |  6 ------
 .../media/platform/marvell-ccic/cafe-driver.c |  7 -------
 drivers/media/platform/omap3isp/isp.c         |  3 ++-
 drivers/media/platform/omap3isp/isph3a_af.c   |  2 +-
 10 files changed, 10 insertions(+), 58 deletions(-)

-- 
2.33.1

[PATCH 01/10] media: cx25821: drop duplicated i2c_slave_did_ack()

[Mauro Carvalho Chehab]

This function is defined twice, one at cx25821-i2c and the other
inside cx25821-core. It turns that only the first
is actually used inside the code.

This causes a clang warning.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/cx25821/cx25821-core.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
index 40c10ca94def..3078a39f0b95 100644
--- a/drivers/media/pci/cx25821/cx25821-core.c
+++ b/drivers/media/pci/cx25821/cx25821-core.c
@@ -337,13 +337,6 @@ static int cx25821_risc_decode(u32 risc)
 	return incr[risc >> 28] ? incr[risc >> 28] : 1;
 }
 
-static inline int i2c_slave_did_ack(struct i2c_adapter *i2c_adap)
-{
-	struct cx25821_i2c *bus = i2c_adap->algo_data;
-	struct cx25821_dev *dev = bus->dev;
-	return cx_read(bus->reg_stat) & 0x01;
-}
-
 static void cx25821_registers_init(struct cx25821_dev *dev)
 {
 	u32 tmp;
-- 
2.33.1

[PATCH 02/10] media: ivtv: drop an unused macro

[Mauro Carvalho Chehab]

The macro p_to_snd_ivtv_card() is just a variant for container_of(),
not actually used inside the code. So, drop it.

This address a clang warning.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/ivtv/ivtv-alsa-main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 4cefdb2e4d40..9e13a7128a53 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -48,12 +48,6 @@ struct snd_ivtv_card *to_snd_ivtv_card(struct v4l2_device *v4l2_dev)
 	return to_ivtv(v4l2_dev)->alsa;
 }
 
-static inline
-struct snd_ivtv_card *p_to_snd_ivtv_card(struct v4l2_device **v4l2_dev)
-{
-	return container_of(v4l2_dev, struct snd_ivtv_card, v4l2_dev);
-}
-
 static void snd_ivtv_card_free(struct snd_ivtv_card *itvsc)
 {
 	if (itvsc == NULL)
-- 
2.33.1

[PATCH 03/10] media: cx18: drop an unused macro

[Mauro Carvalho Chehab]

The macro p_to_snd_cx18_card() is just a variant for container_of(),
not actually used inside the code. So, drop it.

This address a clang warning.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/cx18/cx18-alsa-main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/pci/cx18/cx18-alsa-main.c b/drivers/media/pci/cx18/cx18-alsa-main.c
index 9a82e68303b6..9dc361886284 100644
--- a/drivers/media/pci/cx18/cx18-alsa-main.c
+++ b/drivers/media/pci/cx18/cx18-alsa-main.c
@@ -51,12 +51,6 @@ struct snd_cx18_card *to_snd_cx18_card(struct v4l2_device *v4l2_dev)
 	return to_cx18(v4l2_dev)->alsa;
 }
 
-static inline
-struct snd_cx18_card *p_to_snd_cx18_card(struct v4l2_device **v4l2_dev)
-{
-	return container_of(v4l2_dev, struct snd_cx18_card, v4l2_dev);
-}
-
 static void snd_cx18_card_free(struct snd_cx18_card *cxsc)
 {
 	if (cxsc == NULL)
-- 
2.33.1

[PATCH 04/10] media: stb6100: mark a currently unused function as such

[Mauro Carvalho Chehab]

The stb6100_normalise_regs() function is not used with current
boards, but the driver says that some devices could need it.
Ok, we could simply drop it, but as there's a macro to mark
unused functions, use it.

This should shut up a clang warning.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/stb6100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/stb6100.c b/drivers/media/dvb-frontends/stb6100.c
index d541d6613610..ea1d260e4d70 100644
--- a/drivers/media/dvb-frontends/stb6100.c
+++ b/drivers/media/dvb-frontends/stb6100.c
@@ -110,7 +110,7 @@ static const struct stb6100_regmask stb6100_template[] = {
 /*
  * Currently unused. Some boards might need it in the future
  */
-static inline void stb6100_normalise_regs(u8 regs[])
+static __maybe_unused inline void stb6100_normalise_regs(u8 regs[])
 {
 	int i;
 
-- 
2.33.1

[PATCH 05/10] media: mc: mark a debug function with __maybe_unused

[Mauro Carvalho Chehab]

The gobj_type() function translates the media object type into
a name, being useful for debugging purposes. It is currently
not used, but let's keep it around, as it can be useful.

So, mark it with __maybe_unused, in order to shut up a
clang warning.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/mc/mc-entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index f40f41977142..b0ea145ac9c0 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -14,7 +14,7 @@
 #include <media/media-entity.h>
 #include <media/media-device.h>
 
-static inline const char *gobj_type(enum media_gobj_type type)
+static inline __maybe_unused const char *gobj_type(enum media_gobj_type type)
 {
 	switch (type) {
 	case MEDIA_GRAPH_ENTITY:
-- 
2.33.1

Re: [PATCH 05/10] media: mc: mark a debug function with __maybe_unused

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Mon, Nov 22, 2021 at 09:26:07AM +0000, Mauro Carvalho Chehab wrote:
> The gobj_type() function translates the media object type into
> a name, being useful for debugging purposes. It is currently
> not used, but let's keep it around, as it can be useful.

Don't we usually remove dead code, and add it back later when and if
needed, unless we know the code will be used in the near future ? I
don't care too much either way.

> So, mark it with __maybe_unused, in order to shut up a
> clang warning.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/mc/mc-entity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index f40f41977142..b0ea145ac9c0 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -14,7 +14,7 @@
>  #include <media/media-entity.h>
>  #include <media/media-device.h>
>  
> -static inline const char *gobj_type(enum media_gobj_type type)
> +static inline __maybe_unused const char *gobj_type(enum media_gobj_type type)
>  {
>  	switch (type) {
>  	case MEDIA_GRAPH_ENTITY:

-- 
Regards,

Laurent Pinchart

[PATCH 06/10] media: dvb-core: dvb_frontend: address some clang warnings

[Mauro Carvalho Chehab]

The typecasts at the dvb-core generate clang warnings when W=1
is enabled.

The warns are harmless, but they cause the build to break with
CONFIG_WERROR, so do the cast on a way that it won't produce
warnings anymore.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-core/dvb_frontend.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 8fc59f3cfb6c..48e735cdbe6b 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2554,8 +2554,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
 
 	case FE_DISEQC_SEND_BURST:
 		if (fe->ops.diseqc_send_burst) {
-			err = fe->ops.diseqc_send_burst(fe,
-						(enum fe_sec_mini_cmd)parg);
+			err = fe->ops.diseqc_send_burst(fe, (long)parg);
 			fepriv->state = FESTATE_DISEQC;
 			fepriv->status = 0;
 		}
@@ -2563,9 +2562,8 @@ static int dvb_frontend_handle_ioctl(struct file *file,
 
 	case FE_SET_TONE:
 		if (fe->ops.set_tone) {
-			err = fe->ops.set_tone(fe,
-					       (enum fe_sec_tone_mode)parg);
-			fepriv->tone = (enum fe_sec_tone_mode)parg;
+			fepriv->tone = (long)parg;
+			err = fe->ops.set_tone(fe, fepriv->tone);
 			fepriv->state = FESTATE_DISEQC;
 			fepriv->status = 0;
 		}
@@ -2573,9 +2571,8 @@ static int dvb_frontend_handle_ioctl(struct file *file,
 
 	case FE_SET_VOLTAGE:
 		if (fe->ops.set_voltage) {
-			err = fe->ops.set_voltage(fe,
-						  (enum fe_sec_voltage)parg);
-			fepriv->voltage = (enum fe_sec_voltage)parg;
+			fepriv->voltage = (long)parg;
+			err = fe->ops.set_voltage(fe, fepriv->voltage);
 			fepriv->state = FESTATE_DISEQC;
 			fepriv->status = 0;
 		}
-- 
2.33.1

[PATCH 07/10] media: cx25840: drop some unused inline functions

[Mauro Carvalho Chehab]

Solve some clang warnings about unused functions by dropping
some time-conversion inline funcs that aren't used anywhere.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/i2c/cx25840/cx25840-ir.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-ir.c b/drivers/media/i2c/cx25840/cx25840-ir.c
index 2cf3e6a1f9e1..9d7d1d149f1a 100644
--- a/drivers/media/i2c/cx25840/cx25840-ir.c
+++ b/drivers/media/i2c/cx25840/cx25840-ir.c
@@ -136,19 +136,6 @@ static inline u16 count_to_clock_divider(unsigned int d)
 	return (u16) d;
 }
 
-static inline u16 ns_to_clock_divider(unsigned int ns)
-{
-	return count_to_clock_divider(
-		DIV_ROUND_CLOSEST(CX25840_IR_REFCLK_FREQ / 1000000 * ns, 1000));
-}
-
-static inline unsigned int clock_divider_to_ns(unsigned int divider)
-{
-	/* Period of the Rx or Tx clock in ns */
-	return DIV_ROUND_CLOSEST((divider + 1) * 1000,
-				 CX25840_IR_REFCLK_FREQ / 1000000);
-}
-
 static inline u16 carrier_freq_to_clock_divider(unsigned int freq)
 {
 	return count_to_clock_divider(
@@ -160,13 +147,6 @@ static inline unsigned int clock_divider_to_carrier_freq(unsigned int divider)
 	return DIV_ROUND_CLOSEST(CX25840_IR_REFCLK_FREQ, (divider + 1) * 16);
 }
 
-static inline u16 freq_to_clock_divider(unsigned int freq,
-					unsigned int rollovers)
-{
-	return count_to_clock_divider(
-		   DIV_ROUND_CLOSEST(CX25840_IR_REFCLK_FREQ, freq * rollovers));
-}
-
 static inline unsigned int clock_divider_to_freq(unsigned int divider,
 						 unsigned int rollovers)
 {
-- 
2.33.1

[PATCH 08/10] media: marvell-ccic: drop to_cam() unused function

[Mauro Carvalho Chehab]

This function is an alias for container_of() that it is currently
not used, causing clang warnings. So, drop it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/marvell-ccic/cafe-driver.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c b/drivers/media/platform/marvell-ccic/cafe-driver.c
index b61b9d9551af..03dcf8bf705e 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -139,13 +139,6 @@ struct cafe_camera {
  */
 #define CAFE_SMBUS_TIMEOUT (HZ)  /* generous */
 
-static inline struct cafe_camera *to_cam(struct v4l2_device *dev)
-{
-	struct mcam_camera *m = container_of(dev, struct mcam_camera, v4l2_dev);
-	return container_of(m, struct cafe_camera, mcam);
-}
-
-
 static int cafe_smbus_write_done(struct mcam_camera *mcam)
 {
 	unsigned long flags;
-- 
2.33.1

[PATCH 09/10] media: omap3isp: mark isp_isr_dbg as __maybe_unused

[Mauro Carvalho Chehab]

This function is only used for debugging purposes. When DEBUG
is disabled, it becomes unused, causing a clang warning with W=1.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/omap3isp/isp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6de377ce281d..4c937f3f323e 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -476,7 +476,8 @@ void omap3isp_hist_dma_done(struct isp_device *isp)
 	}
 }
 
-static inline void isp_isr_dbg(struct isp_device *isp, u32 irqstatus)
+static inline void __maybe_unused isp_isr_dbg(struct isp_device *isp,
+					      u32 irqstatus)
 {
 	static const char *name[] = {
 		"CSIA_IRQ",
-- 
2.33.1

Re: [PATCH 09/10] media: omap3isp: mark isp_isr_dbg as __maybe_unused

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Mon, Nov 22, 2021 at 09:26:11AM +0000, Mauro Carvalho Chehab wrote:
> This function is only used for debugging purposes. When DEBUG
> is disabled, it becomes unused, causing a clang warning with W=1.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/platform/omap3isp/isp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index 6de377ce281d..4c937f3f323e 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -476,7 +476,8 @@ void omap3isp_hist_dma_done(struct isp_device *isp)
>  	}
>  }
>  
> -static inline void isp_isr_dbg(struct isp_device *isp, u32 irqstatus)
> +static inline void __maybe_unused isp_isr_dbg(struct isp_device *isp,
> +					      u32 irqstatus)
>  {
>  	static const char *name[] = {
>  		"CSIA_IRQ",

-- 
Regards,

Laurent Pinchart

[PATCH v2 09/10] media: mc: drop an unused debug function

[Mauro Carvalho Chehab]

The gobj_type() function translates the media object type into
a name, being useful for debugging purposes. It is currently
not used. So, drop it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/mc/mc-entity.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index f40f41977142..c02340698ad6 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -14,22 +14,6 @@
 #include <media/media-entity.h>
 #include <media/media-device.h>
 
-static inline const char *gobj_type(enum media_gobj_type type)
-{
-	switch (type) {
-	case MEDIA_GRAPH_ENTITY:
-		return "entity";
-	case MEDIA_GRAPH_PAD:
-		return "pad";
-	case MEDIA_GRAPH_LINK:
-		return "link";
-	case MEDIA_GRAPH_INTF_DEVNODE:
-		return "intf-devnode";
-	default:
-		return "unknown";
-	}
-}
-
 static inline const char *intf_type(struct media_interface *intf)
 {
 	switch (intf->type) {
-- 
2.33.1

[PATCH 10/10] media: omap3isp: avoid warnings at IS_OUT_OF_BOUNDS()

[Mauro Carvalho Chehab]

Clang with W=1 produces the following warnings:

drivers/media/platform/omap3isp/isph3a_af.c:173:6: error: result of comparison of constant 256 with expression of type '__u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (IS_OUT_OF_BOUNDS(paxel_cfg->height, OMAP3ISP_AF_PAXEL_HEIGHT_MIN,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isph3a_af.c:24:33: note: expanded from macro 'IS_OUT_OF_BOUNDS'
        (((value) < (min)) || ((value) > (max)))
                               ~~~~~~~ ^ ~~~~~
drivers/media/platform/omap3isp/isph3a_af.c:179:6: error: result of comparison of constant 256 with expression of type '__u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (IS_OUT_OF_BOUNDS(paxel_cfg->width, OMAP3ISP_AF_PAXEL_WIDTH_MIN,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isph3a_af.c:24:33: note: expanded from macro 'IS_OUT_OF_BOUNDS'
        (((value) < (min)) || ((value) > (max)))
                               ~~~~~~~ ^ ~~~~~
2 errors generated.

Use a typecast to avoid such warnings.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/omap3isp/isph3a_af.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c
index a65cfdfa9637..de7b116d0122 100644
--- a/drivers/media/platform/omap3isp/isph3a_af.c
+++ b/drivers/media/platform/omap3isp/isph3a_af.c
@@ -21,7 +21,7 @@
 #include "ispstat.h"
 
 #define IS_OUT_OF_BOUNDS(value, min, max)		\
-	(((value) < (min)) || ((value) > (max)))
+	((((unsigned int)value) < (min)) || (((unsigned int)value) > (max)))
 
 static void h3a_af_setup_regs(struct ispstat *af, void *priv)
 {
-- 
2.33.1

Re: [PATCH 10/10] media: omap3isp: avoid warnings at IS_OUT_OF_BOUNDS()

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Mon, Nov 22, 2021 at 09:26:12AM +0000, Mauro Carvalho Chehab wrote:
> Clang with W=1 produces the following warnings:
> 
> drivers/media/platform/omap3isp/isph3a_af.c:173:6: error: result of comparison of constant 256 with expression of type '__u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (IS_OUT_OF_BOUNDS(paxel_cfg->height, OMAP3ISP_AF_PAXEL_HEIGHT_MIN,
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/media/platform/omap3isp/isph3a_af.c:24:33: note: expanded from macro 'IS_OUT_OF_BOUNDS'
>         (((value) < (min)) || ((value) > (max)))
>                                ~~~~~~~ ^ ~~~~~
> drivers/media/platform/omap3isp/isph3a_af.c:179:6: error: result of comparison of constant 256 with expression of type '__u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (IS_OUT_OF_BOUNDS(paxel_cfg->width, OMAP3ISP_AF_PAXEL_WIDTH_MIN,
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/media/platform/omap3isp/isph3a_af.c:24:33: note: expanded from macro 'IS_OUT_OF_BOUNDS'
>         (((value) < (min)) || ((value) > (max)))
>                                ~~~~~~~ ^ ~~~~~
> 2 errors generated.
> 
> Use a typecast to avoid such warnings.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/10] at: https://lore.kernel.org/all/cover.1637573097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/platform/omap3isp/isph3a_af.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c
> index a65cfdfa9637..de7b116d0122 100644
> --- a/drivers/media/platform/omap3isp/isph3a_af.c
> +++ b/drivers/media/platform/omap3isp/isph3a_af.c
> @@ -21,7 +21,7 @@
>  #include "ispstat.h"
>  
>  #define IS_OUT_OF_BOUNDS(value, min, max)		\
> -	(((value) < (min)) || ((value) > (max)))
> +	((((unsigned int)value) < (min)) || (((unsigned int)value) > (max)))

It could have been a good occasion to replace the macro with an inline
function.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  
>  static void h3a_af_setup_regs(struct ispstat *af, void *priv)
>  {

-- 
Regards,

Laurent Pinchart