[PATCH 00/20] Solve the remaining issues with clang and W=1 on media

[PATCH 00/20] Solve the remaining issues with clang and W=1 on media

[Mauro Carvalho Chehab]

Currently, using clang with W=1 and CONFIG_WERROR causes media to break.
This is reported by builder.linuxtv.org.

This series solve the remaining issues.

Mauro Carvalho Chehab (20):
  media: adv7842: get rid of two unused functions
  media: saa7134-go7007: get rid of to_state() function
  media: davinci: get rid of an unused function
  media: drxd: drop offset var from DownloadMicrocode()
  media: drxk: drop operation_mode from set_dvbt()
  media: m88ds3103: drop reg11 calculus from m88ds3103b_select_mclk()
  media: si21xx: report eventual errors at set_frontend
  media: solo6x10: add _maybe_unused to currently unused functions
  media: si470x: fix printk warnings with clang
  media: radio-si476x: drop a container_of() abstraction macro
  media: lmedm04: don't ignore errors when setting a filter
  media: au0828-i2c: drop a duplicated function
  media: adv7604 add _maybe_unused to currently unused functions
  media: adv7511: drop unused functions
  media: imx290: mark read reg function as __maybe_unused
  media: davinci: vpbe_osd: mark read reg function as __maybe_unused
  media: qcom: camss: mark read reg function as  __maybe_unused
  media: mtk-mdp: address a clang warning
  media: cobalt: drop an unused variable
  media: mxl5005s: drop some dead code

 drivers/media/dvb-frontends/drxd_hard.c       |  8 -------
 drivers/media/dvb-frontends/drxk_hard.c       |  6 -----
 drivers/media/dvb-frontends/m88ds3103.c       |  6 +----
 drivers/media/dvb-frontends/si21xx.c          |  7 +++---
 drivers/media/i2c/adv7511-v4l2.c              | 22 -------------------
 drivers/media/i2c/adv7604.c                   | 18 +++++++--------
 drivers/media/i2c/adv7842.c                   | 10 ---------
 drivers/media/i2c/imx290.c                    |  2 +-
 drivers/media/pci/cobalt/cobalt-cpld.c        |  5 +----
 drivers/media/pci/saa7134/saa7134-go7007.c    |  7 +-----
 .../media/pci/solo6x10/solo6x10-v4l2-enc.c    | 12 +++++-----
 drivers/media/platform/davinci/vpbe_osd.c     |  2 +-
 drivers/media/platform/davinci/vpif_capture.c | 11 ----------
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c |  2 +-
 .../media/platform/qcom/camss/camss-vfe-170.c |  2 +-
 drivers/media/radio/radio-si476x.c            |  6 -----
 drivers/media/radio/si470x/radio-si470x-i2c.c |  4 ++--
 drivers/media/radio/si470x/radio-si470x-usb.c |  8 +++----
 drivers/media/tuners/mxl5005s.c               | 14 +-----------
 drivers/media/usb/au0828/au0828-i2c.c         |  7 ------
 drivers/media/usb/dvb-usb-v2/lmedm04.c        |  3 +++
 21 files changed, 36 insertions(+), 126 deletions(-)

-- 
2.33.1

[PATCH 01/20] media: adv7842: get rid of two unused functions

[Mauro Carvalho Chehab]

Those functions just call a V4L2 core macro, and aren't used
anywhere. Get rid of them, in order to avoid clang warnings 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/i2c/adv7842.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 7f8acbdf0db4..9d6eed0f8281 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -256,21 +256,11 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
 	return &container_of(ctrl->handler, struct adv7842_state, hdl)->sd;
 }
 
-static inline unsigned hblanking(const struct v4l2_bt_timings *t)
-{
-	return V4L2_DV_BT_BLANKING_WIDTH(t);
-}
-
 static inline unsigned htotal(const struct v4l2_bt_timings *t)
 {
 	return V4L2_DV_BT_FRAME_WIDTH(t);
 }
 
-static inline unsigned vblanking(const struct v4l2_bt_timings *t)
-{
-	return V4L2_DV_BT_BLANKING_HEIGHT(t);
-}
-
 static inline unsigned vtotal(const struct v4l2_bt_timings *t)
 {
 	return V4L2_DV_BT_FRAME_HEIGHT(t);
-- 
2.33.1

Re: [PATCH 01/20] media: adv7842: get rid of two unused functions

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:04PM +0100, Mauro Carvalho Chehab wrote:
> Those functions just call a V4L2 core macro, and aren't used
> anywhere. Get rid of them, in order to avoid clang warnings with
> W=1.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/i2c/adv7842.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
> index 7f8acbdf0db4..9d6eed0f8281 100644
> --- a/drivers/media/i2c/adv7842.c
> +++ b/drivers/media/i2c/adv7842.c
> @@ -256,21 +256,11 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
>  	return &container_of(ctrl->handler, struct adv7842_state, hdl)->sd;
>  }
>  
> -static inline unsigned hblanking(const struct v4l2_bt_timings *t)
> -{
> -	return V4L2_DV_BT_BLANKING_WIDTH(t);
> -}
> -
>  static inline unsigned htotal(const struct v4l2_bt_timings *t)
>  {
>  	return V4L2_DV_BT_FRAME_WIDTH(t);
>  }
>  
> -static inline unsigned vblanking(const struct v4l2_bt_timings *t)
> -{
> -	return V4L2_DV_BT_BLANKING_HEIGHT(t);
> -}
> -
>  static inline unsigned vtotal(const struct v4l2_bt_timings *t)
>  {
>  	return V4L2_DV_BT_FRAME_HEIGHT(t);
> -- 
> 2.33.1
> 
> 

[PATCH 02/20] media: saa7134-go7007: get rid of to_state() function

[Mauro Carvalho Chehab]

This is just an alias for container_of() and it is used only
on a commented code. The commented code actually is an issue that
require fixes, so, move the container_of() call to the commented
code, and drop the unused function.

This way, a fix for the function could be added later without
needing to re-add to_state() function.

This fixes a clang W=1 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/saa7134/saa7134-go7007.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-go7007.c b/drivers/media/pci/saa7134/saa7134-go7007.c
index f319edb39c0e..da83893ffee9 100644
--- a/drivers/media/pci/saa7134/saa7134-go7007.c
+++ b/drivers/media/pci/saa7134/saa7134-go7007.c
@@ -56,11 +56,6 @@ struct saa7134_go7007 {
 	dma_addr_t bottom_dma;
 };
 
-static inline struct saa7134_go7007 *to_state(struct v4l2_subdev *sd)
-{
-	return container_of(sd, struct saa7134_go7007, sd);
-}
-
 static const struct go7007_board_info board_voyager = {
 	.flags		 = 0,
 	.sensor_flags	 = GO7007_SENSOR_656 |
@@ -385,7 +380,7 @@ MODULE_FIRMWARE("go7007/go7007tv.bin");
 static int saa7134_go7007_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
 {
 #if 0
-	struct saa7134_go7007 *saa = to_state(sd);
+	struct saa7134_go7007 *saa = container_of(sd, struct saa7134_go7007, sd);
 	struct saa7134_dev *dev = saa->dev;
 
 	return saa7134_s_std_internal(dev, NULL, norm);
-- 
2.33.1

Re: [PATCH 02/20] media: saa7134-go7007: get rid of to_state() function

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:05PM +0100, Mauro Carvalho Chehab wrote:
> This is just an alias for container_of() and it is used only
> on a commented code. The commented code actually is an issue that
> require fixes, so, move the container_of() call to the commented
> code, and drop the unused function.
> 
> This way, a fix for the function could be added later without
> needing to re-add to_state() function.
> 
> This fixes a clang W=1 warning.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/pci/saa7134/saa7134-go7007.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-go7007.c b/drivers/media/pci/saa7134/saa7134-go7007.c
> index f319edb39c0e..da83893ffee9 100644
> --- a/drivers/media/pci/saa7134/saa7134-go7007.c
> +++ b/drivers/media/pci/saa7134/saa7134-go7007.c
> @@ -56,11 +56,6 @@ struct saa7134_go7007 {
>  	dma_addr_t bottom_dma;
>  };
>  
> -static inline struct saa7134_go7007 *to_state(struct v4l2_subdev *sd)
> -{
> -	return container_of(sd, struct saa7134_go7007, sd);
> -}
> -
>  static const struct go7007_board_info board_voyager = {
>  	.flags		 = 0,
>  	.sensor_flags	 = GO7007_SENSOR_656 |
> @@ -385,7 +380,7 @@ MODULE_FIRMWARE("go7007/go7007tv.bin");
>  static int saa7134_go7007_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
>  {
>  #if 0
> -	struct saa7134_go7007 *saa = to_state(sd);
> +	struct saa7134_go7007 *saa = container_of(sd, struct saa7134_go7007, sd);
>  	struct saa7134_dev *dev = saa->dev;
>  
>  	return saa7134_s_std_internal(dev, NULL, norm);
> -- 
> 2.33.1
> 
> 

[PATCH 03/20] media: davinci: get rid of an unused function

[Mauro Carvalho Chehab]

vpif_get_default_field() seems to be some left-over from a
past code that sets the field order.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/davinci/vpif_capture.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index ae92e2c206d0..aba105fa7ef9 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -618,17 +618,6 @@ static void vpif_calculate_offsets(struct channel_obj *ch)
 	ch->vpifparams.video_params.stdid = vpifparams->std_info.stdid;
 }
 
-/**
- * vpif_get_default_field() - Get default field type based on interface
- * @iface: ptr to vpif interface
- */
-static inline enum v4l2_field vpif_get_default_field(
-				struct vpif_interface *iface)
-{
-	return (iface->if_type == VPIF_IF_RAW_BAYER) ? V4L2_FIELD_NONE :
-						V4L2_FIELD_INTERLACED;
-}
-
 /**
  * vpif_config_addr() - function to configure buffer address in vpif
  * @ch: channel ptr
-- 
2.33.1

Re: [PATCH 03/20] media: davinci: get rid of an unused function

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:06PM +0100, Mauro Carvalho Chehab wrote:
> vpif_get_default_field() seems to be some left-over from a
> past code that sets the field order.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/platform/davinci/vpif_capture.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index ae92e2c206d0..aba105fa7ef9 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -618,17 +618,6 @@ static void vpif_calculate_offsets(struct channel_obj *ch)
>  	ch->vpifparams.video_params.stdid = vpifparams->std_info.stdid;
>  }
>  
> -/**
> - * vpif_get_default_field() - Get default field type based on interface
> - * @iface: ptr to vpif interface
> - */
> -static inline enum v4l2_field vpif_get_default_field(
> -				struct vpif_interface *iface)
> -{
> -	return (iface->if_type == VPIF_IF_RAW_BAYER) ? V4L2_FIELD_NONE :
> -						V4L2_FIELD_INTERLACED;
> -}
> -
>  /**
>   * vpif_config_addr() - function to configure buffer address in vpif
>   * @ch: channel ptr
> -- 
> 2.33.1
> 
> 

[PATCH 04/20] media: drxd: drop offset var from DownloadMicrocode()

[Mauro Carvalho Chehab]

The offset is not needed, and it is never used, as the pointer
itself is already incremented.

So, drop it, in order to solve a W=1 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/drxd_hard.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/drxd_hard.c b/drivers/media/dvb-frontends/drxd_hard.c
index a7eb81df88c2..9860cae65f1c 100644
--- a/drivers/media/dvb-frontends/drxd_hard.c
+++ b/drivers/media/dvb-frontends/drxd_hard.c
@@ -914,44 +914,36 @@ static int DownloadMicrocode(struct drxd_state *state,
 	u32 Address;
 	u16 nBlocks;
 	u16 BlockSize;
-	u32 offset = 0;
 	int i, status = 0;
 
 	pSrc = (u8 *) pMCImage;
 	/* We're not using Flags */
 	/* Flags = (pSrc[0] << 8) | pSrc[1]; */
 	pSrc += sizeof(u16);
-	offset += sizeof(u16);
 	nBlocks = (pSrc[0] << 8) | pSrc[1];
 	pSrc += sizeof(u16);
-	offset += sizeof(u16);
 
 	for (i = 0; i < nBlocks; i++) {
 		Address = (pSrc[0] << 24) | (pSrc[1] << 16) |
 		    (pSrc[2] << 8) | pSrc[3];
 		pSrc += sizeof(u32);
-		offset += sizeof(u32);
 
 		BlockSize = ((pSrc[0] << 8) | pSrc[1]) * sizeof(u16);
 		pSrc += sizeof(u16);
-		offset += sizeof(u16);
 
 		/* We're not using Flags */
 		/* u16 Flags = (pSrc[0] << 8) | pSrc[1]; */
 		pSrc += sizeof(u16);
-		offset += sizeof(u16);
 
 		/* We're not using BlockCRC */
 		/* u16 BlockCRC = (pSrc[0] << 8) | pSrc[1]; */
 		pSrc += sizeof(u16);
-		offset += sizeof(u16);
 
 		status = WriteBlock(state, Address, BlockSize,
 				    pSrc, DRX_I2C_CLEARCRC);
 		if (status < 0)
 			break;
 		pSrc += BlockSize;
-		offset += BlockSize;
 	}
 
 	return status;
-- 
2.33.1

Re: [PATCH 04/20] media: drxd: drop offset var from DownloadMicrocode()

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:07PM +0100, Mauro Carvalho Chehab wrote:
> The offset is not needed, and it is never used, as the pointer
> itself is already incremented.
> 
> So, drop it, in order to solve a W=1 clang warning.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

I would expect GCC to warn about this as well.

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/dvb-frontends/drxd_hard.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/drxd_hard.c b/drivers/media/dvb-frontends/drxd_hard.c
> index a7eb81df88c2..9860cae65f1c 100644
> --- a/drivers/media/dvb-frontends/drxd_hard.c
> +++ b/drivers/media/dvb-frontends/drxd_hard.c
> @@ -914,44 +914,36 @@ static int DownloadMicrocode(struct drxd_state *state,
>  	u32 Address;
>  	u16 nBlocks;
>  	u16 BlockSize;
> -	u32 offset = 0;
>  	int i, status = 0;
>  
>  	pSrc = (u8 *) pMCImage;
>  	/* We're not using Flags */
>  	/* Flags = (pSrc[0] << 8) | pSrc[1]; */
>  	pSrc += sizeof(u16);
> -	offset += sizeof(u16);
>  	nBlocks = (pSrc[0] << 8) | pSrc[1];
>  	pSrc += sizeof(u16);
> -	offset += sizeof(u16);
>  
>  	for (i = 0; i < nBlocks; i++) {
>  		Address = (pSrc[0] << 24) | (pSrc[1] << 16) |
>  		    (pSrc[2] << 8) | pSrc[3];
>  		pSrc += sizeof(u32);
> -		offset += sizeof(u32);
>  
>  		BlockSize = ((pSrc[0] << 8) | pSrc[1]) * sizeof(u16);
>  		pSrc += sizeof(u16);
> -		offset += sizeof(u16);
>  
>  		/* We're not using Flags */
>  		/* u16 Flags = (pSrc[0] << 8) | pSrc[1]; */
>  		pSrc += sizeof(u16);
> -		offset += sizeof(u16);
>  
>  		/* We're not using BlockCRC */
>  		/* u16 BlockCRC = (pSrc[0] << 8) | pSrc[1]; */
>  		pSrc += sizeof(u16);
> -		offset += sizeof(u16);
>  
>  		status = WriteBlock(state, Address, BlockSize,
>  				    pSrc, DRX_I2C_CLEARCRC);
>  		if (status < 0)
>  			break;
>  		pSrc += BlockSize;
> -		offset += BlockSize;
>  	}
>  
>  	return status;
> -- 
> 2.33.1
> 
> 

[PATCH 05/20] media: drxk: drop operation_mode from set_dvbt()

[Mauro Carvalho Chehab]

This var is set, but never used. 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/drxk_hard.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
index d7fc2595f15b..afa0ac85313f 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -3720,7 +3720,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 {
 	u16 cmd_result = 0;
 	u16 transmission_params = 0;
-	u16 operation_mode = 0;
 	u32 iqm_rc_rate_ofs = 0;
 	u32 bandwidth = 0;
 	u16 param1;
@@ -3760,7 +3759,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.transmission_mode) {
 	case TRANSMISSION_MODE_AUTO:
 	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_MODE__M;
 		fallthrough;	/* try first guess DRX_FFTMODE_8K */
 	case TRANSMISSION_MODE_8K:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_MODE_8K;
@@ -3774,7 +3772,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.guard_interval) {
 	default:
 	case GUARD_INTERVAL_AUTO:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_GUARD__M;
 		fallthrough;	/* try first guess DRX_GUARD_1DIV4 */
 	case GUARD_INTERVAL_1_4:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_4;
@@ -3795,7 +3792,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	case HIERARCHY_AUTO:
 	case HIERARCHY_NONE:
 	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_HIER__M;
 		/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
 		/* transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_NO; */
 		fallthrough;
@@ -3815,7 +3811,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.modulation) {
 	case QAM_AUTO:
 	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_CONST__M;
 		fallthrough;	/* try first guess DRX_CONSTELLATION_QAM64 */
 	case QAM_64:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_CONST_QAM64;
@@ -3858,7 +3853,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.code_rate_HP) {
 	case FEC_AUTO:
 	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_RATE__M;
 		fallthrough;	/* try first guess DRX_CODERATE_2DIV3 */
 	case FEC_2_3:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_RATE_2_3;
-- 
2.33.1

Re: [PATCH 05/20] media: drxk: drop operation_mode from set_dvbt()

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:08PM +0100, Mauro Carvalho Chehab wrote:
> This var is set, but never used. 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/dvb-frontends/drxk_hard.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
> index d7fc2595f15b..afa0ac85313f 100644
> --- a/drivers/media/dvb-frontends/drxk_hard.c
> +++ b/drivers/media/dvb-frontends/drxk_hard.c
> @@ -3720,7 +3720,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  {
>  	u16 cmd_result = 0;
>  	u16 transmission_params = 0;
> -	u16 operation_mode = 0;
>  	u32 iqm_rc_rate_ofs = 0;
>  	u32 bandwidth = 0;
>  	u16 param1;
> @@ -3760,7 +3759,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  	switch (state->props.transmission_mode) {
>  	case TRANSMISSION_MODE_AUTO:
>  	default:
> -		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_MODE__M;
>  		fallthrough;	/* try first guess DRX_FFTMODE_8K */

I think that all these fallthrough annotations in these blocks are
useless now and can be eliminated, as the block contains no code to
fallthrough on.

>  	case TRANSMISSION_MODE_8K:
>  		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_MODE_8K;
> @@ -3774,7 +3772,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  	switch (state->props.guard_interval) {
>  	default:
>  	case GUARD_INTERVAL_AUTO:
> -		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_GUARD__M;
>  		fallthrough;	/* try first guess DRX_GUARD_1DIV4 */
>  	case GUARD_INTERVAL_1_4:
>  		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_4;
> @@ -3795,7 +3792,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  	case HIERARCHY_AUTO:
>  	case HIERARCHY_NONE:
>  	default:
> -		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_HIER__M;
>  		/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
>  		/* transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_NO; */
>  		fallthrough;
> @@ -3815,7 +3811,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  	switch (state->props.modulation) {
>  	case QAM_AUTO:
>  	default:
> -		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_CONST__M;
>  		fallthrough;	/* try first guess DRX_CONSTELLATION_QAM64 */
>  	case QAM_64:
>  		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_CONST_QAM64;
> @@ -3858,7 +3853,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
>  	switch (state->props.code_rate_HP) {
>  	case FEC_AUTO:
>  	default:
> -		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_RATE__M;
>  		fallthrough;	/* try first guess DRX_CODERATE_2DIV3 */
>  	case FEC_2_3:
>  		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_RATE_2_3;
> -- 
> 2.33.1
> 
> 

[PATCH v2 05/20] media: drxk: drop operation_mode from set_dvbt()

[Mauro Carvalho Chehab]

This var is set, but never used. So, drop it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/dvb-frontends/drxk_hard.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
index d7fc2595f15b..9430295a8175 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -3720,7 +3720,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 {
 	u16 cmd_result = 0;
 	u16 transmission_params = 0;
-	u16 operation_mode = 0;
 	u32 iqm_rc_rate_ofs = 0;
 	u32 bandwidth = 0;
 	u16 param1;
@@ -3759,10 +3758,8 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* mode */
 	switch (state->props.transmission_mode) {
 	case TRANSMISSION_MODE_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_MODE__M;
-		fallthrough;	/* try first guess DRX_FFTMODE_8K */
 	case TRANSMISSION_MODE_8K:
+	default:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_MODE_8K;
 		break;
 	case TRANSMISSION_MODE_2K:
@@ -3773,9 +3770,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* guard */
 	switch (state->props.guard_interval) {
 	default:
-	case GUARD_INTERVAL_AUTO:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_GUARD__M;
-		fallthrough;	/* try first guess DRX_GUARD_1DIV4 */
+	case GUARD_INTERVAL_AUTO: /* try first guess DRX_GUARD_1DIV4 */
 	case GUARD_INTERVAL_1_4:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_4;
 		break;
@@ -3794,11 +3789,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.hierarchy) {
 	case HIERARCHY_AUTO:
 	case HIERARCHY_NONE:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_HIER__M;
-		/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
-		/* transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_NO; */
-		fallthrough;
+	default:	/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
 	case HIERARCHY_1:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_A1;
 		break;
@@ -3814,9 +3805,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* modulation */
 	switch (state->props.modulation) {
 	case QAM_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_CONST__M;
-		fallthrough;	/* try first guess DRX_CONSTELLATION_QAM64 */
+	default:	/* try first guess DRX_CONSTELLATION_QAM64 */
 	case QAM_64:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_CONST_QAM64;
 		break;
@@ -3857,9 +3846,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* coderate */
 	switch (state->props.code_rate_HP) {
 	case FEC_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_RATE__M;
-		fallthrough;	/* try first guess DRX_CODERATE_2DIV3 */
+	default:	/* try first guess DRX_CODERATE_2DIV3 */
 	case FEC_2_3:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_RATE_2_3;
 		break;
-- 
2.33.1

[PATCH 06/20] media: m88ds3103: drop reg11 calculus from m88ds3103b_select_mclk()

[Mauro Carvalho Chehab]

This vaklue is never used at the code. So, drop it.

Solves a W=1 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/m88ds3103.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 02e8aa11e36e..bce0f42f3d19 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -451,7 +451,7 @@ static int m88ds3103b_select_mclk(struct m88ds3103_dev *dev)
 
 static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
 {
-	u8 reg11 = 0x0A, reg15, reg16, reg1D, reg1E, reg1F, tmp;
+	u8 reg15, reg16, reg1D, reg1E, reg1F, tmp;
 	u8 sm, f0 = 0, f1 = 0, f2 = 0, f3 = 0;
 	u16 pll_div_fb, N;
 	u32 div;
@@ -480,8 +480,6 @@ static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
 	div /= mclk_khz;
 
 	if (dev->cfg->ts_mode == M88DS3103_TS_SERIAL) {
-		reg11 |= 0x02;
-
 		if (div <= 32) {
 			N = 2;
 
@@ -532,8 +530,6 @@ static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
 		else if ((f3 < 8) && (f3 != 0))
 			f3 = 8;
 	} else {
-		reg11 &= ~0x02;
-
 		if (div <= 32) {
 			N = 2;
 
-- 
2.33.1

Re: [PATCH 06/20] media: m88ds3103: drop reg11 calculus from m88ds3103b_select_mclk()

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:09PM +0100, Mauro Carvalho Chehab wrote:
> This vaklue is never used at the code. So, drop it.

       ^ value

> Solves a W=1 clang warning.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

I would expect GCC to warn about this as well.

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/dvb-frontends/m88ds3103.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
> index 02e8aa11e36e..bce0f42f3d19 100644
> --- a/drivers/media/dvb-frontends/m88ds3103.c
> +++ b/drivers/media/dvb-frontends/m88ds3103.c
> @@ -451,7 +451,7 @@ static int m88ds3103b_select_mclk(struct m88ds3103_dev *dev)
>  
>  static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
>  {
> -	u8 reg11 = 0x0A, reg15, reg16, reg1D, reg1E, reg1F, tmp;
> +	u8 reg15, reg16, reg1D, reg1E, reg1F, tmp;
>  	u8 sm, f0 = 0, f1 = 0, f2 = 0, f3 = 0;
>  	u16 pll_div_fb, N;
>  	u32 div;
> @@ -480,8 +480,6 @@ static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
>  	div /= mclk_khz;
>  
>  	if (dev->cfg->ts_mode == M88DS3103_TS_SERIAL) {
> -		reg11 |= 0x02;
> -
>  		if (div <= 32) {
>  			N = 2;
>  
> @@ -532,8 +530,6 @@ static int m88ds3103b_set_mclk(struct m88ds3103_dev *dev, u32 mclk_khz)
>  		else if ((f3 < 8) && (f3 != 0))
>  			f3 = 8;
>  	} else {
> -		reg11 &= ~0x02;
> -
>  		if (div <= 32) {
>  			N = 2;
>  
> -- 
> 2.33.1
> 
> 

[PATCH 07/20] media: si21xx: report eventual errors at set_frontend

[Mauro Carvalho Chehab]

If an error occurs while setting the registers at set_frontend,
it is silently ignored. Yet, the variable status is updated.

Change the logic to return an error if it fails to write values
to the registers.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/si21xx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/si21xx.c b/drivers/media/dvb-frontends/si21xx.c
index e31eb2c5cc4c..001b23588389 100644
--- a/drivers/media/dvb-frontends/si21xx.c
+++ b/drivers/media/dvb-frontends/si21xx.c
@@ -711,7 +711,7 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
 	int i;
 	bool inband_interferer_div2[ALLOWABLE_FS_COUNT];
 	bool inband_interferer_div4[ALLOWABLE_FS_COUNT];
-	int status;
+	int status = 0;
 
 	/* allowable sample rates for ADC in MHz */
 	int afs[ALLOWABLE_FS_COUNT] = { 200, 192, 193, 194, 195,
@@ -747,8 +747,6 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
 	rf_freq = 10 * c->frequency ;
 	data_rate = c->symbol_rate / 100;
 
-	status = PASS;
-
 	band_low = (rf_freq - lnb_lo) - ((lnb_uncertanity * 200)
 					+ (data_rate * 135)) / 200;
 
@@ -832,6 +830,9 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
 	state->fs = sample_rate;/*ADC MHz*/
 	si21xx_setacquire(fe, c->symbol_rate, c->fec_inner);
 
+	if (status)
+		return -EREMOTEIO;
+
 	return 0;
 }
 
-- 
2.33.1

Re: [PATCH 07/20] media: si21xx: report eventual errors at set_frontend

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:10PM +0100, Mauro Carvalho Chehab wrote:
> If an error occurs while setting the registers at set_frontend,
> it is silently ignored. Yet, the variable status is updated.
> 
> Change the logic to return an error if it fails to write values
> to the registers.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

I am not super familiar with the different return codes so I assume it
is appropriate (the sites that I see calling set_frontend() appears to
only check for a negative return code).

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/dvb-frontends/si21xx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/si21xx.c b/drivers/media/dvb-frontends/si21xx.c
> index e31eb2c5cc4c..001b23588389 100644
> --- a/drivers/media/dvb-frontends/si21xx.c
> +++ b/drivers/media/dvb-frontends/si21xx.c
> @@ -711,7 +711,7 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
>  	int i;
>  	bool inband_interferer_div2[ALLOWABLE_FS_COUNT];
>  	bool inband_interferer_div4[ALLOWABLE_FS_COUNT];
> -	int status;
> +	int status = 0;
>  
>  	/* allowable sample rates for ADC in MHz */
>  	int afs[ALLOWABLE_FS_COUNT] = { 200, 192, 193, 194, 195,
> @@ -747,8 +747,6 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
>  	rf_freq = 10 * c->frequency ;
>  	data_rate = c->symbol_rate / 100;
>  
> -	status = PASS;
> -
>  	band_low = (rf_freq - lnb_lo) - ((lnb_uncertanity * 200)
>  					+ (data_rate * 135)) / 200;
>  
> @@ -832,6 +830,9 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
>  	state->fs = sample_rate;/*ADC MHz*/
>  	si21xx_setacquire(fe, c->symbol_rate, c->fec_inner);
>  
> +	if (status)
> +		return -EREMOTEIO;
> +
>  	return 0;
>  }
>  
> -- 
> 2.33.1
> 
> 

Re: [PATCH 07/20] media: si21xx: report eventual errors at set_frontend

[Mauro Carvalho Chehab]

Em Fri, 26 Nov 2021 12:45:23 -0700
Nathan Chancellor <nathan@kernel.org> escreveu:

> On Wed, Nov 24, 2021 at 08:13:10PM +0100, Mauro Carvalho Chehab wrote:
> > If an error occurs while setting the registers at set_frontend,
> > it is silently ignored. Yet, the variable status is updated.
> > 
> > Change the logic to return an error if it fails to write values
> > to the registers.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>  
> 
> I am not super familiar with the different return codes so I assume it
> is appropriate (the sites that I see calling set_frontend() appears to
> only check for a negative return code).

There are a couple of return codes used on media when I/O transfer fails.
Some places use -EREMOTEIO. Others use -EIO or -ENXIO.

I guess -EREMOTEIO is a little bit better than -EIO for I2C transfers,
as the problem usually happened inside an I2C bus at the DVB adapter.

> 
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 
> > ---
> > 
> > To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> > See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> > 
> >  drivers/media/dvb-frontends/si21xx.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/dvb-frontends/si21xx.c b/drivers/media/dvb-frontends/si21xx.c
> > index e31eb2c5cc4c..001b23588389 100644
> > --- a/drivers/media/dvb-frontends/si21xx.c
> > +++ b/drivers/media/dvb-frontends/si21xx.c
> > @@ -711,7 +711,7 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
> >  	int i;
> >  	bool inband_interferer_div2[ALLOWABLE_FS_COUNT];
> >  	bool inband_interferer_div4[ALLOWABLE_FS_COUNT];
> > -	int status;
> > +	int status = 0;
> >  
> >  	/* allowable sample rates for ADC in MHz */
> >  	int afs[ALLOWABLE_FS_COUNT] = { 200, 192, 193, 194, 195,
> > @@ -747,8 +747,6 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
> >  	rf_freq = 10 * c->frequency ;
> >  	data_rate = c->symbol_rate / 100;
> >  
> > -	status = PASS;
> > -
> >  	band_low = (rf_freq - lnb_lo) - ((lnb_uncertanity * 200)
> >  					+ (data_rate * 135)) / 200;
> >  
> > @@ -832,6 +830,9 @@ static int si21xx_set_frontend(struct dvb_frontend *fe)
> >  	state->fs = sample_rate;/*ADC MHz*/
> >  	si21xx_setacquire(fe, c->symbol_rate, c->fec_inner);
> >  
> > +	if (status)
> > +		return -EREMOTEIO;
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.33.1
> > 
> >   



Thanks,
Mauro

[PATCH 08/20] media: solo6x10: add _maybe_unused to currently unused functions

[Mauro Carvalho Chehab]

There are several unused helper macros there, meant to parse some
fields.

While there's not wrong with that, it generates clang warnings
with W=1, causing build to break with CONFIG_WERROR.

So, add __maybe_unused to fix 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 0abcad4e84fa..85eaf5d00e9b 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -391,12 +391,12 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
 }
 
 /* Extract values from VOP header - VE_STATUSxx */
-static inline int vop_interlaced(const vop_header *vh)
+static inline __maybe_unused int vop_interlaced(const vop_header *vh)
 {
 	return (__le32_to_cpu((*vh)[0]) >> 30) & 1;
 }
 
-static inline u8 vop_channel(const vop_header *vh)
+static inline __maybe_unused u8 vop_channel(const vop_header *vh)
 {
 	return (__le32_to_cpu((*vh)[0]) >> 24) & 0x1F;
 }
@@ -411,12 +411,12 @@ static inline u32 vop_mpeg_size(const vop_header *vh)
 	return __le32_to_cpu((*vh)[0]) & 0xFFFFF;
 }
 
-static inline u8 vop_hsize(const vop_header *vh)
+static inline u8 __maybe_unused vop_hsize(const vop_header *vh)
 {
 	return (__le32_to_cpu((*vh)[1]) >> 8) & 0xFF;
 }
 
-static inline u8 vop_vsize(const vop_header *vh)
+static inline u8 __maybe_unused vop_vsize(const vop_header *vh)
 {
 	return __le32_to_cpu((*vh)[1]) & 0xFF;
 }
@@ -436,12 +436,12 @@ static inline u32 vop_jpeg_size(const vop_header *vh)
 	return __le32_to_cpu((*vh)[4]) & 0xFFFFF;
 }
 
-static inline u32 vop_sec(const vop_header *vh)
+static inline u32 __maybe_unused vop_sec(const vop_header *vh)
 {
 	return __le32_to_cpu((*vh)[5]);
 }
 
-static inline u32 vop_usec(const vop_header *vh)
+static inline __maybe_unused u32 vop_usec(const vop_header *vh)
 {
 	return __le32_to_cpu((*vh)[6]);
 }
-- 
2.33.1

Re: [PATCH 08/20] media: solo6x10: add _maybe_unused to currently unused functions

[Ismael Luceno]

On 24/Nov/2021 20:13, Mauro Carvalho Chehab wrote:
> There are several unused helper macros there, meant to parse some
> fields.
> 
> While there's not wrong with that, it generates clang warnings
> with W=1, causing build to break with CONFIG_WERROR.
> 
> So, add __maybe_unused to fix 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> index 0abcad4e84fa..85eaf5d00e9b 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> @@ -391,12 +391,12 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
>  }
>  
>  /* Extract values from VOP header - VE_STATUSxx */
> -static inline int vop_interlaced(const vop_header *vh)
> +static inline __maybe_unused int vop_interlaced(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[0]) >> 30) & 1;
>  }
>  
> -static inline u8 vop_channel(const vop_header *vh)
> +static inline __maybe_unused u8 vop_channel(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[0]) >> 24) & 0x1F;
>  }
> @@ -411,12 +411,12 @@ static inline u32 vop_mpeg_size(const vop_header *vh)
>  	return __le32_to_cpu((*vh)[0]) & 0xFFFFF;
>  }
>  
> -static inline u8 vop_hsize(const vop_header *vh)
> +static inline u8 __maybe_unused vop_hsize(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[1]) >> 8) & 0xFF;
>  }
>  
> -static inline u8 vop_vsize(const vop_header *vh)
> +static inline u8 __maybe_unused vop_vsize(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[1]) & 0xFF;
>  }
> @@ -436,12 +436,12 @@ static inline u32 vop_jpeg_size(const vop_header *vh)
>  	return __le32_to_cpu((*vh)[4]) & 0xFFFFF;
>  }
>  
> -static inline u32 vop_sec(const vop_header *vh)
> +static inline u32 __maybe_unused vop_sec(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[5]);
>  }
>  
> -static inline u32 vop_usec(const vop_header *vh)
> +static inline __maybe_unused u32 vop_usec(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[6]);
>  }
> -- 
> 2.33.1
> 

Acked-by: Ismael Luceno <ismael@iodev.co.uk>

Re: [PATCH 08/20] media: solo6x10: add _maybe_unused to currently unused functions

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:11PM +0100, Mauro Carvalho Chehab wrote:
> There are several unused helper macros there, meant to parse some
> fields.
> 
> While there's not wrong with that, it generates clang warnings
> with W=1, causing build to break with CONFIG_WERROR.
> 
> So, add __maybe_unused to fix such warnings.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

I'll comment on this one patch but my opinion applies for all the
patches adding '__maybe_unused' to truly unused functions.

I agree with Laurent's comment [1]: unless this code is going to be used
soon, it should be deleted and resurrected when it is actually needed.
We have git for a reason and by adding this attribute, you are making it
harder to catch and eliminate unused functions, as no compiler will
catch them with an unused attribute (it is possible other static
analysis tools will but I doubt those are run as frequently as compilers
with W=1).

However, you are the maintainer so if you really want to keep these
around, I would recommend adding '__always_unused' instead of
'__maybe_unused' for documentation and auditing purposes, even though
they evaluate to the same thing:

$ rg __always_unused | wc -l
337

$ rg __maybe_unused | wc -l
4335

[1]: https://lore.kernel.org/r/YZtpnjPcGxVwhe61@pendragon.ideasonboard.com/

Cheers,
Nathan

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> index 0abcad4e84fa..85eaf5d00e9b 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> @@ -391,12 +391,12 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
>  }
>  
>  /* Extract values from VOP header - VE_STATUSxx */
> -static inline int vop_interlaced(const vop_header *vh)
> +static inline __maybe_unused int vop_interlaced(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[0]) >> 30) & 1;
>  }
>  
> -static inline u8 vop_channel(const vop_header *vh)
> +static inline __maybe_unused u8 vop_channel(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[0]) >> 24) & 0x1F;
>  }
> @@ -411,12 +411,12 @@ static inline u32 vop_mpeg_size(const vop_header *vh)
>  	return __le32_to_cpu((*vh)[0]) & 0xFFFFF;
>  }
>  
> -static inline u8 vop_hsize(const vop_header *vh)
> +static inline u8 __maybe_unused vop_hsize(const vop_header *vh)
>  {
>  	return (__le32_to_cpu((*vh)[1]) >> 8) & 0xFF;
>  }
>  
> -static inline u8 vop_vsize(const vop_header *vh)
> +static inline u8 __maybe_unused vop_vsize(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[1]) & 0xFF;
>  }
> @@ -436,12 +436,12 @@ static inline u32 vop_jpeg_size(const vop_header *vh)
>  	return __le32_to_cpu((*vh)[4]) & 0xFFFFF;
>  }
>  
> -static inline u32 vop_sec(const vop_header *vh)
> +static inline u32 __maybe_unused vop_sec(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[5]);
>  }
>  
> -static inline u32 vop_usec(const vop_header *vh)
> +static inline __maybe_unused u32 vop_usec(const vop_header *vh)
>  {
>  	return __le32_to_cpu((*vh)[6]);
>  }
> -- 
> 2.33.1
> 
> 

Re: [PATCH 08/20] media: solo6x10: add _maybe_unused to currently unused functions

[Mauro Carvalho Chehab]

Em Fri, 26 Nov 2021 12:58:33 -0700
Nathan Chancellor <nathan@kernel.org> escreveu:

> On Wed, Nov 24, 2021 at 08:13:11PM +0100, Mauro Carvalho Chehab wrote:
> > There are several unused helper macros there, meant to parse some
> > fields.
> > 
> > While there's not wrong with that, it generates clang warnings
> > with W=1, causing build to break with CONFIG_WERROR.
> > 
> > So, add __maybe_unused to fix such warnings.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>  
> 
> I'll comment on this one patch but my opinion applies for all the
> patches adding '__maybe_unused' to truly unused functions.
> 
> I agree with Laurent's comment [1]: unless this code is going to be used
> soon, it should be deleted and resurrected when it is actually needed.
> We have git for a reason and by adding this attribute, you are making it
> harder to catch and eliminate unused functions, as no compiler will
> catch them with an unused attribute (it is possible other static
> analysis tools will but I doubt those are run as frequently as compilers
> with W=1).

In this specific case (and on a few other patches on this series), those
macros are used to document a field. That's why I opted to keep it.

> 
> However, you are the maintainer so if you really want to keep these
> around, I would recommend adding '__always_unused' instead of
> '__maybe_unused' for documentation and auditing purposes, even though
> they evaluate to the same thing:
> 
> $ rg __always_unused | wc -l
> 337
> 
> $ rg __maybe_unused | wc -l
> 4335

Good point. I'll use __always_unused on such patches.

> 
> [1]: https://lore.kernel.org/r/YZtpnjPcGxVwhe61@pendragon.ideasonboard.com/
> 
> Cheers,
> Nathan
> 
> > ---
> > 
> > To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> > See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> > 
> >  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > index 0abcad4e84fa..85eaf5d00e9b 100644
> > --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > @@ -391,12 +391,12 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
> >  }
> >  
> >  /* Extract values from VOP header - VE_STATUSxx */
> > -static inline int vop_interlaced(const vop_header *vh)
> > +static inline __maybe_unused int vop_interlaced(const vop_header *vh)
> >  {
> >  	return (__le32_to_cpu((*vh)[0]) >> 30) & 1;
> >  }
> >  
> > -static inline u8 vop_channel(const vop_header *vh)
> > +static inline __maybe_unused u8 vop_channel(const vop_header *vh)
> >  {
> >  	return (__le32_to_cpu((*vh)[0]) >> 24) & 0x1F;
> >  }
> > @@ -411,12 +411,12 @@ static inline u32 vop_mpeg_size(const vop_header *vh)
> >  	return __le32_to_cpu((*vh)[0]) & 0xFFFFF;
> >  }
> >  
> > -static inline u8 vop_hsize(const vop_header *vh)
> > +static inline u8 __maybe_unused vop_hsize(const vop_header *vh)
> >  {
> >  	return (__le32_to_cpu((*vh)[1]) >> 8) & 0xFF;
> >  }
> >  
> > -static inline u8 vop_vsize(const vop_header *vh)
> > +static inline u8 __maybe_unused vop_vsize(const vop_header *vh)
> >  {
> >  	return __le32_to_cpu((*vh)[1]) & 0xFF;
> >  }
> > @@ -436,12 +436,12 @@ static inline u32 vop_jpeg_size(const vop_header *vh)
> >  	return __le32_to_cpu((*vh)[4]) & 0xFFFFF;
> >  }
> >  
> > -static inline u32 vop_sec(const vop_header *vh)
> > +static inline u32 __maybe_unused vop_sec(const vop_header *vh)
> >  {
> >  	return __le32_to_cpu((*vh)[5]);
> >  }
> >  
> > -static inline u32 vop_usec(const vop_header *vh)
> > +static inline __maybe_unused u32 vop_usec(const vop_header *vh)
> >  {
> >  	return __le32_to_cpu((*vh)[6]);
> >  }
> > -- 
> > 2.33.1
> > 
> >   



Thanks,
Mauro

[PATCH 09/20] media: si470x: fix printk warnings with clang

[Mauro Carvalho Chehab]

Clang doesn't like "%hu" on macros:

	drivers/media/radio/si470x/radio-si470x-i2c.c:414:4: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
	drivers/media/radio/si470x/radio-si470x-i2c.c:417:4: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]

So, just replace them with "%u".

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/radio/si470x/radio-si470x-i2c.c | 4 ++--
 drivers/media/radio/si470x/radio-si470x-usb.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index a972c0705ac7..7ea7c6326f53 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -410,10 +410,10 @@ static int si470x_i2c_probe(struct i2c_client *client)
 			radio->registers[DEVICEID], radio->registers[SI_CHIPID]);
 	if ((radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE) < RADIO_FW_VERSION) {
 		dev_warn(&client->dev,
-			"This driver is known to work with firmware version %hu,\n",
+			"This driver is known to work with firmware version %u,\n",
 			RADIO_FW_VERSION);
 		dev_warn(&client->dev,
-			"but the device has firmware version %hu.\n",
+			"but the device has firmware version %u.\n",
 			radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE);
 		version_warning = 1;
 	}
diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index 3f8634a46573..1e70e6971fe4 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -681,10 +681,10 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 			radio->registers[DEVICEID], radio->registers[SI_CHIPID]);
 	if ((radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE) < RADIO_FW_VERSION) {
 		dev_warn(&intf->dev,
-			"This driver is known to work with firmware version %hu,\n",
+			"This driver is known to work with firmware version %u,\n",
 			RADIO_FW_VERSION);
 		dev_warn(&intf->dev,
-			"but the device has firmware version %hu.\n",
+			"but the device has firmware version %u.\n",
 			radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE);
 		version_warning = 1;
 	}
@@ -698,10 +698,10 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 			radio->software_version, radio->hardware_version);
 	if (radio->hardware_version < RADIO_HW_VERSION) {
 		dev_warn(&intf->dev,
-			"This driver is known to work with hardware version %hu,\n",
+			"This driver is known to work with hardware version %u,\n",
 			RADIO_HW_VERSION);
 		dev_warn(&intf->dev,
-			"but the device has hardware version %hu.\n",
+			"but the device has hardware version %u.\n",
 			radio->hardware_version);
 		version_warning = 1;
 	}
-- 
2.33.1

Re: [PATCH 09/20] media: si470x: fix printk warnings with clang

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:12PM +0100, Mauro Carvalho Chehab wrote:
> Clang doesn't like "%hu" on macros:
> 
> 	drivers/media/radio/si470x/radio-si470x-i2c.c:414:4: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
> 	drivers/media/radio/si470x/radio-si470x-i2c.c:417:4: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
> 
> So, just replace them with "%u".
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Maybe worth mentioning commit cbacb5ab0aa0 ("docs: printk-formats: Stop
encouraging use of unnecessary %h[xudi] and %hh[xudi]")?

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/radio/si470x/radio-si470x-i2c.c | 4 ++--
>  drivers/media/radio/si470x/radio-si470x-usb.c | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
> index a972c0705ac7..7ea7c6326f53 100644
> --- a/drivers/media/radio/si470x/radio-si470x-i2c.c
> +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
> @@ -410,10 +410,10 @@ static int si470x_i2c_probe(struct i2c_client *client)
>  			radio->registers[DEVICEID], radio->registers[SI_CHIPID]);
>  	if ((radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE) < RADIO_FW_VERSION) {
>  		dev_warn(&client->dev,
> -			"This driver is known to work with firmware version %hu,\n",
> +			"This driver is known to work with firmware version %u,\n",
>  			RADIO_FW_VERSION);
>  		dev_warn(&client->dev,
> -			"but the device has firmware version %hu.\n",
> +			"but the device has firmware version %u.\n",
>  			radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE);
>  		version_warning = 1;
>  	}
> diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
> index 3f8634a46573..1e70e6971fe4 100644
> --- a/drivers/media/radio/si470x/radio-si470x-usb.c
> +++ b/drivers/media/radio/si470x/radio-si470x-usb.c
> @@ -681,10 +681,10 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
>  			radio->registers[DEVICEID], radio->registers[SI_CHIPID]);
>  	if ((radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE) < RADIO_FW_VERSION) {
>  		dev_warn(&intf->dev,
> -			"This driver is known to work with firmware version %hu,\n",
> +			"This driver is known to work with firmware version %u,\n",
>  			RADIO_FW_VERSION);
>  		dev_warn(&intf->dev,
> -			"but the device has firmware version %hu.\n",
> +			"but the device has firmware version %u.\n",
>  			radio->registers[SI_CHIPID] & SI_CHIPID_FIRMWARE);
>  		version_warning = 1;
>  	}
> @@ -698,10 +698,10 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
>  			radio->software_version, radio->hardware_version);
>  	if (radio->hardware_version < RADIO_HW_VERSION) {
>  		dev_warn(&intf->dev,
> -			"This driver is known to work with hardware version %hu,\n",
> +			"This driver is known to work with hardware version %u,\n",
>  			RADIO_HW_VERSION);
>  		dev_warn(&intf->dev,
> -			"but the device has hardware version %hu.\n",
> +			"but the device has hardware version %u.\n",
>  			radio->hardware_version);
>  		version_warning = 1;
>  	}
> -- 
> 2.33.1
> 
> 

[PATCH 10/20] media: radio-si476x: drop a container_of() abstraction macro

[Mauro Carvalho Chehab]

This isn't used anywhere. 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/radio/radio-si476x.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/radio/radio-si476x.c b/drivers/media/radio/radio-si476x.c
index b39a68f83c5f..0bf99e1cd1d8 100644
--- a/drivers/media/radio/radio-si476x.c
+++ b/drivers/media/radio/radio-si476x.c
@@ -312,12 +312,6 @@ struct si476x_radio {
 	u32 audmode;
 };
 
-static inline struct si476x_radio *
-v4l2_dev_to_radio(struct v4l2_device *d)
-{
-	return container_of(d, struct si476x_radio, v4l2dev);
-}
-
 static inline struct si476x_radio *
 v4l2_ctrl_handler_to_radio(struct v4l2_ctrl_handler *d)
 {
-- 
2.33.1

Re: [PATCH 10/20] media: radio-si476x: drop a container_of() abstraction macro

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:13PM +0100, Mauro Carvalho Chehab wrote:
> This isn't used anywhere. So, drop it.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/radio/radio-si476x.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/media/radio/radio-si476x.c b/drivers/media/radio/radio-si476x.c
> index b39a68f83c5f..0bf99e1cd1d8 100644
> --- a/drivers/media/radio/radio-si476x.c
> +++ b/drivers/media/radio/radio-si476x.c
> @@ -312,12 +312,6 @@ struct si476x_radio {
>  	u32 audmode;
>  };
>  
> -static inline struct si476x_radio *
> -v4l2_dev_to_radio(struct v4l2_device *d)
> -{
> -	return container_of(d, struct si476x_radio, v4l2dev);
> -}
> -
>  static inline struct si476x_radio *
>  v4l2_ctrl_handler_to_radio(struct v4l2_ctrl_handler *d)
>  {
> -- 
> 2.33.1
> 
> 

[PATCH 11/20] media: lmedm04: don't ignore errors when setting a filter

[Mauro Carvalho Chehab]

Solves 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/usb/dvb-usb-v2/lmedm04.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
index fe4d886442a4..8a34e6c0d6a6 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -423,6 +423,9 @@ static int lme2510_pid_filter_ctrl(struct dvb_usb_adapter *adap, int onoff)
 
 	mutex_unlock(&d->i2c_mutex);
 
+	if (ret)
+		return -EREMOTEIO;
+
 	return 0;
 }
 
-- 
2.33.1

Re: [PATCH 11/20] media: lmedm04: don't ignore errors when setting a filter

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:14PM +0100, Mauro Carvalho Chehab wrote:
> Solves a clang warning
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/usb/dvb-usb-v2/lmedm04.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> index fe4d886442a4..8a34e6c0d6a6 100644
> --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> @@ -423,6 +423,9 @@ static int lme2510_pid_filter_ctrl(struct dvb_usb_adapter *adap, int onoff)
>  
>  	mutex_unlock(&d->i2c_mutex);
>  
> +	if (ret)
> +		return -EREMOTEIO;
> +
>  	return 0;
>  }
>  
> -- 
> 2.33.1
> 
> 

[PATCH 12/20] media: au0828-i2c: drop a duplicated function

[Mauro Carvalho Chehab]

This function is not used and it is identical to
i2c_slave_did_read_ack(). So, just 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/usb/au0828/au0828-i2c.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-i2c.c b/drivers/media/usb/au0828/au0828-i2c.c
index 708f01ab47fa..749f90d73b5b 100644
--- a/drivers/media/usb/au0828/au0828-i2c.c
+++ b/drivers/media/usb/au0828/au0828-i2c.c
@@ -23,13 +23,6 @@ MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
 #define I2C_WAIT_DELAY 25
 #define I2C_WAIT_RETRY 1000
 
-static inline int i2c_slave_did_write_ack(struct i2c_adapter *i2c_adap)
-{
-	struct au0828_dev *dev = i2c_adap->algo_data;
-	return au0828_read(dev, AU0828_I2C_STATUS_201) &
-		AU0828_I2C_STATUS_NO_WRITE_ACK ? 0 : 1;
-}
-
 static inline int i2c_slave_did_read_ack(struct i2c_adapter *i2c_adap)
 {
 	struct au0828_dev *dev = i2c_adap->algo_data;
-- 
2.33.1

Re: [PATCH 12/20] media: au0828-i2c: drop a duplicated function

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:15PM +0100, Mauro Carvalho Chehab wrote:
> This function is not used and it is identical to
> i2c_slave_did_read_ack(). So, just drop it.

Not quite, i2c_slave_did_write_ack() checks against
AU0828_I2C_STATUS_NO_WRITE_ACK, whereas i2c_slave_did_read_ack() checks
against AU0828_I2C_STATUS_NO_READ_ACK.

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

Regardless of that, it is unused and I do not see anywhere obvious where
it should be.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/usb/au0828/au0828-i2c.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/media/usb/au0828/au0828-i2c.c b/drivers/media/usb/au0828/au0828-i2c.c
> index 708f01ab47fa..749f90d73b5b 100644
> --- a/drivers/media/usb/au0828/au0828-i2c.c
> +++ b/drivers/media/usb/au0828/au0828-i2c.c
> @@ -23,13 +23,6 @@ MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
>  #define I2C_WAIT_DELAY 25
>  #define I2C_WAIT_RETRY 1000
>  
> -static inline int i2c_slave_did_write_ack(struct i2c_adapter *i2c_adap)
> -{
> -	struct au0828_dev *dev = i2c_adap->algo_data;
> -	return au0828_read(dev, AU0828_I2C_STATUS_201) &
> -		AU0828_I2C_STATUS_NO_WRITE_ACK ? 0 : 1;
> -}
> -
>  static inline int i2c_slave_did_read_ack(struct i2c_adapter *i2c_adap)
>  {
>  	struct au0828_dev *dev = i2c_adap->algo_data;
> -- 
> 2.33.1
> 
> 

[PATCH 13/20] media: adv7604 add _maybe_unused to currently unused functions

[Mauro Carvalho Chehab]

There are several unused helper macros there, meant to parse some
fields.

While there's not wrong with that, it generates clang warnings
with W=1, causing build to break with CONFIG_WERROR.

So, add __maybe_unused to fix 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/i2c/adv7604.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 44768b59a6ff..be95b2264221 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -398,14 +398,14 @@ static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask,
 	return io_write(sd, reg, (io_read(sd, reg) & ~mask) | val);
 }
 
-static inline int avlink_read(struct v4l2_subdev *sd, u8 reg)
+static inline int __maybe_unused avlink_read(struct v4l2_subdev *sd, u8 reg)
 {
 	struct adv76xx_state *state = to_state(sd);
 
 	return adv76xx_read_check(state, ADV7604_PAGE_AVLINK, reg);
 }
 
-static inline int avlink_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+static inline int __maybe_unused avlink_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
 	struct adv76xx_state *state = to_state(sd);
 
@@ -439,14 +439,14 @@ static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg)
 	return adv76xx_read_check(state, ADV76XX_PAGE_INFOFRAME, reg);
 }
 
-static inline int infoframe_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+static inline int __maybe_unused infoframe_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
 	struct adv76xx_state *state = to_state(sd);
 
 	return regmap_write(state->regmap[ADV76XX_PAGE_INFOFRAME], reg, val);
 }
 
-static inline int afe_read(struct v4l2_subdev *sd, u8 reg)
+static inline int __maybe_unused afe_read(struct v4l2_subdev *sd, u8 reg)
 {
 	struct adv76xx_state *state = to_state(sd);
 
@@ -479,14 +479,14 @@ static inline int rep_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8
 	return rep_write(sd, reg, (rep_read(sd, reg) & ~mask) | val);
 }
 
-static inline int edid_read(struct v4l2_subdev *sd, u8 reg)
+static inline int __maybe_unused edid_read(struct v4l2_subdev *sd, u8 reg)
 {
 	struct adv76xx_state *state = to_state(sd);
 
 	return adv76xx_read_check(state, ADV76XX_PAGE_EDID, reg);
 }
 
-static inline int edid_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+static inline int __maybe_unused edid_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
 	struct adv76xx_state *state = to_state(sd);
 
@@ -570,7 +570,7 @@ static inline int hdmi_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8
 	return hdmi_write(sd, reg, (hdmi_read(sd, reg) & ~mask) | val);
 }
 
-static inline int test_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+static inline int __maybe_unused test_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
 	struct adv76xx_state *state = to_state(sd);
 
@@ -601,14 +601,14 @@ static inline int cp_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask, u8 v
 	return cp_write(sd, reg, (cp_read(sd, reg) & ~mask) | val);
 }
 
-static inline int vdp_read(struct v4l2_subdev *sd, u8 reg)
+static inline int __maybe_unused vdp_read(struct v4l2_subdev *sd, u8 reg)
 {
 	struct adv76xx_state *state = to_state(sd);
 
 	return adv76xx_read_check(state, ADV7604_PAGE_VDP, reg);
 }
 
-static inline int vdp_write(struct v4l2_subdev *sd, u8 reg, u8 val)
+static inline int __maybe_unused vdp_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
 	struct adv76xx_state *state = to_state(sd);
 
-- 
2.33.1

[PATCH 14/20] media: adv7511: drop unused functions

[Mauro Carvalho Chehab]

Those are aliases for another function and not used at the
current implementation. So, just 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/i2c/adv7511-v4l2.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/media/i2c/adv7511-v4l2.c b/drivers/media/i2c/adv7511-v4l2.c
index 41f4e749a859..8e13cae40ec5 100644
--- a/drivers/media/i2c/adv7511-v4l2.c
+++ b/drivers/media/i2c/adv7511-v4l2.c
@@ -270,28 +270,6 @@ static int adv7511_pktmem_rd(struct v4l2_subdev *sd, u8 reg)
 	return adv_smbus_read_byte_data(state->i2c_pktmem, reg);
 }
 
-static int adv7511_pktmem_wr(struct v4l2_subdev *sd, u8 reg, u8 val)
-{
-	struct adv7511_state *state = get_adv7511_state(sd);
-	int ret;
-	int i;
-
-	for (i = 0; i < 3; i++) {
-		ret = i2c_smbus_write_byte_data(state->i2c_pktmem, reg, val);
-		if (ret == 0)
-			return 0;
-	}
-	v4l2_err(sd, "%s: i2c write error\n", __func__);
-	return ret;
-}
-
-/* To set specific bits in the register, a clear-mask is given (to be AND-ed),
-   and then the value-mask (to be OR-ed). */
-static inline void adv7511_pktmem_wr_and_or(struct v4l2_subdev *sd, u8 reg, u8 clr_mask, u8 val_mask)
-{
-	adv7511_pktmem_wr(sd, reg, (adv7511_pktmem_rd(sd, reg) & clr_mask) | val_mask);
-}
-
 static inline bool adv7511_have_hotplug(struct v4l2_subdev *sd)
 {
 	return adv7511_rd(sd, 0x42) & MASK_ADV7511_HPD_DETECT;
-- 
2.33.1

Re: [PATCH 14/20] media: adv7511: drop unused functions

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:17PM +0100, Mauro Carvalho Chehab wrote:
> Those are aliases for another function and not used at the
> current implementation. So, just drop it.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/i2c/adv7511-v4l2.c | 22 ----------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7511-v4l2.c b/drivers/media/i2c/adv7511-v4l2.c
> index 41f4e749a859..8e13cae40ec5 100644
> --- a/drivers/media/i2c/adv7511-v4l2.c
> +++ b/drivers/media/i2c/adv7511-v4l2.c
> @@ -270,28 +270,6 @@ static int adv7511_pktmem_rd(struct v4l2_subdev *sd, u8 reg)
>  	return adv_smbus_read_byte_data(state->i2c_pktmem, reg);
>  }
>  
> -static int adv7511_pktmem_wr(struct v4l2_subdev *sd, u8 reg, u8 val)
> -{
> -	struct adv7511_state *state = get_adv7511_state(sd);
> -	int ret;
> -	int i;
> -
> -	for (i = 0; i < 3; i++) {
> -		ret = i2c_smbus_write_byte_data(state->i2c_pktmem, reg, val);
> -		if (ret == 0)
> -			return 0;
> -	}
> -	v4l2_err(sd, "%s: i2c write error\n", __func__);
> -	return ret;
> -}
> -
> -/* To set specific bits in the register, a clear-mask is given (to be AND-ed),
> -   and then the value-mask (to be OR-ed). */
> -static inline void adv7511_pktmem_wr_and_or(struct v4l2_subdev *sd, u8 reg, u8 clr_mask, u8 val_mask)
> -{
> -	adv7511_pktmem_wr(sd, reg, (adv7511_pktmem_rd(sd, reg) & clr_mask) | val_mask);
> -}
> -
>  static inline bool adv7511_have_hotplug(struct v4l2_subdev *sd)
>  {
>  	return adv7511_rd(sd, 0x42) & MASK_ADV7511_HPD_DETECT;
> -- 
> 2.33.1
> 
> 

[PATCH 15/20] media: imx290: mark read reg function as __maybe_unused

[Mauro Carvalho Chehab]

Such function is currently unused, but could be needed in the
future. So, keep it, marking it as __maybe_unused, in order to
avoid a clang W=1 error.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

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

diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
index bf7a6c37ca5d..caab830f8c72 100644
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -363,7 +363,7 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
 	return container_of(_sd, struct imx290, sd);
 }
 
-static inline int imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
+static inline int __maybe_unused imx290_read_reg(struct imx290 *imx290, u16 addr, u8 *value)
 {
 	unsigned int regval;
 	int ret;
-- 
2.33.1

[PATCH 16/20] media: davinci: vpbe_osd: mark read reg function as __maybe_unused

[Mauro Carvalho Chehab]

Such function is currently unused, but could be needed in the
future. So, keep it, marking it as __maybe_unused, in order to
avoid a clang W=1 error.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

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

diff --git a/drivers/media/platform/davinci/vpbe_osd.c b/drivers/media/platform/davinci/vpbe_osd.c
index 91b571a0ac2c..6b36cae5e6c1 100644
--- a/drivers/media/platform/davinci/vpbe_osd.c
+++ b/drivers/media/platform/davinci/vpbe_osd.c
@@ -45,7 +45,7 @@ static const struct platform_device_id vpbe_osd_devtype[] = {
 MODULE_DEVICE_TABLE(platform, vpbe_osd_devtype);
 
 /* register access routines */
-static inline u32 osd_read(struct osd_state *sd, u32 offset)
+static inline u32 __maybe_unused osd_read(struct osd_state *sd, u32 offset)
 {
 	struct osd_state *osd = sd;
 
-- 
2.33.1

[PATCH 17/20] media: qcom: camss: mark read reg function as __maybe_unused

[Mauro Carvalho Chehab]

Such function is currently unused, but could be needed in the
future. So, keep it, marking it as __maybe_unused, in order to
avoid a clang W=1 error.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/qcom/camss/camss-vfe-170.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
index 5c083d70d495..455ae1a819e6 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
@@ -191,7 +191,7 @@ static u32 vfe_hw_version(struct vfe_device *vfe)
 	return hw_version;
 }
 
-static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
+static inline void __maybe_unused vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
 {
 	u32 bits = readl_relaxed(vfe->base + reg);
 
-- 
2.33.1

Re: [PATCH 17/20] media: qcom: camss: mark read reg function as __maybe_unused

[Robert Foss]

Thanks for submitting this Mauro.

On Wed, 24 Nov 2021 at 20:13, Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Such function is currently unused, but could be needed in the
> future. So, keep it, marking it as __maybe_unused, in order to
> avoid a clang W=1 error.
>
> 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
>
>  drivers/media/platform/qcom/camss/camss-vfe-170.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
> index 5c083d70d495..455ae1a819e6 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
> @@ -191,7 +191,7 @@ static u32 vfe_hw_version(struct vfe_device *vfe)
>         return hw_version;
>  }
>
> -static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
> +static inline void __maybe_unused vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
>  {
>         u32 bits = readl_relaxed(vfe->base + reg);
>
> --
> 2.33.1
>

I've submitted a patch[1] for removing this function, with that patch
applied, this patch is no longer needed.

[1] https://lore.kernel.org/all/20211011121301.421410-1-robert.foss@linaro.org/

[PATCH 18/20] media: mtk-mdp: address a clang warning

[Mauro Carvalho Chehab]

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

Such warning is harmless, but it causes the build to break with
CONFIG_WERROR and W=1 with clang, 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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 976aa1f4829b..11285717577f 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -148,7 +148,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 			continue;
 		}
 
-		comp_type = (enum mtk_mdp_comp_type)of_id->data;
+		comp_type = (long)of_id->data;
 
 		comp = devm_kzalloc(dev, sizeof(*comp), GFP_KERNEL);
 		if (!comp) {
-- 
2.33.1

Re: [PATCH 18/20] media: mtk-mdp: address a clang warning

[Arnd Bergmann]

On Wed, Nov 24, 2021 at 8:13 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> The typecasts at the dvb-core generate clang warnings when W=1
> is enabled.
>
> Such warning is harmless, but it causes the build to break with
> CONFIG_WERROR and W=1 with clang, so do the cast on a way that
> it won't produce warnings anymore.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

>
> -               comp_type = (enum mtk_mdp_comp_type)of_id->data;
> +               comp_type = (long)of_id->data;

I would generally use (uintptr_t) for this purpose, but the effect is the same.

        Arnd

[PATCH 19/20] media: cobalt: drop an unused variable

[Mauro Carvalho Chehab]

The clock_control value is read but never actually used. Based on
a comment at the code, it has to be reset at the function.

So, drop the variable that stores its value.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/pci/cobalt/cobalt-cpld.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-cpld.c b/drivers/media/pci/cobalt/cobalt-cpld.c
index 3d8026483ac3..fad882459d23 100644
--- a/drivers/media/pci/cobalt/cobalt-cpld.c
+++ b/drivers/media/pci/cobalt/cobalt-cpld.c
@@ -236,7 +236,6 @@ bool cobalt_cpld_set_freq(struct cobalt *cobalt, unsigned f_out)
 	u8 n1, hsdiv;
 	u8 regs[6];
 	int found = 0;
-	u16 clock_ctrl;
 	int retries = 3;
 
 	for (i = 0; i < ARRAY_SIZE(multipliers); i++) {
@@ -260,9 +259,7 @@ bool cobalt_cpld_set_freq(struct cobalt *cobalt, unsigned f_out)
 	hsdiv = multipliers[i_best].hsdiv - 4;
 	rfreq = div_u64(dco << 28, f_xtal);
 
-	clock_ctrl = cpld_read(cobalt, SI570_CLOCK_CTRL);
-	clock_ctrl |= S01755_REG_CLOCK_CTRL_BITMAP_CLKHSMA_FPGA_CTRL;
-	clock_ctrl |= S01755_REG_CLOCK_CTRL_BITMAP_CLKHSMA_EN;
+	cpld_read(cobalt, SI570_CLOCK_CTRL);
 
 	regs[0] = (hsdiv << 5) | (n1 >> 2);
 	regs[1] = ((n1 & 0x3) << 6) | (rfreq >> 32);
-- 
2.33.1

Re: [PATCH 19/20] media: cobalt: drop an unused variable

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:22PM +0100, Mauro Carvalho Chehab wrote:
> The clock_control value is read but never actually used. Based on
> a comment at the code, it has to be reset at the function.
> 
> So, drop the variable that stores its value.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/pci/cobalt/cobalt-cpld.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/media/pci/cobalt/cobalt-cpld.c b/drivers/media/pci/cobalt/cobalt-cpld.c
> index 3d8026483ac3..fad882459d23 100644
> --- a/drivers/media/pci/cobalt/cobalt-cpld.c
> +++ b/drivers/media/pci/cobalt/cobalt-cpld.c
> @@ -236,7 +236,6 @@ bool cobalt_cpld_set_freq(struct cobalt *cobalt, unsigned f_out)
>  	u8 n1, hsdiv;
>  	u8 regs[6];
>  	int found = 0;
> -	u16 clock_ctrl;
>  	int retries = 3;
>  
>  	for (i = 0; i < ARRAY_SIZE(multipliers); i++) {
> @@ -260,9 +259,7 @@ bool cobalt_cpld_set_freq(struct cobalt *cobalt, unsigned f_out)
>  	hsdiv = multipliers[i_best].hsdiv - 4;
>  	rfreq = div_u64(dco << 28, f_xtal);
>  
> -	clock_ctrl = cpld_read(cobalt, SI570_CLOCK_CTRL);
> -	clock_ctrl |= S01755_REG_CLOCK_CTRL_BITMAP_CLKHSMA_FPGA_CTRL;
> -	clock_ctrl |= S01755_REG_CLOCK_CTRL_BITMAP_CLKHSMA_EN;
> +	cpld_read(cobalt, SI570_CLOCK_CTRL);
>  
>  	regs[0] = (hsdiv << 5) | (n1 >> 2);
>  	regs[1] = ((n1 & 0x3) << 6) | (rfreq >> 32);
> -- 
> 2.33.1
> 
> 

[PATCH 20/20] media: mxl5005s: drop some dead code

[Mauro Carvalho Chehab]

As reported by clang (with W=1), the ctrlVal var is never used.

Yet, there are even some loops to estimate it. As this is dead
code, remove it. If ever needed, someone could revert this
patch.

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/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/

 drivers/media/tuners/mxl5005s.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/media/tuners/mxl5005s.c b/drivers/media/tuners/mxl5005s.c
index f6e82a8e7d37..ab4c43df9d18 100644
--- a/drivers/media/tuners/mxl5005s.c
+++ b/drivers/media/tuners/mxl5005s.c
@@ -3414,9 +3414,8 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
 	u32 value, u16 controlGroup)
 {
 	struct mxl5005s_state *state = fe->tuner_priv;
-	u16 i, j, k;
+	u16 i, j;
 	u32 highLimit;
-	u32 ctrlVal;
 
 	if (controlGroup == 1) /* Initial Control */ {
 
@@ -3432,9 +3431,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
 							(u8)(state->Init_Ctrl[i].bit[j]),
 							(u8)((value>>j) & 0x01));
 					}
-					ctrlVal = 0;
-					for (k = 0; k < state->Init_Ctrl[i].size; k++)
-						ctrlVal += state->Init_Ctrl[i].val[k] * (1 << k);
 				} else
 					return -1;
 			}
@@ -3454,9 +3450,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
 							(u8)(state->CH_Ctrl[i].bit[j]),
 							(u8)((value>>j) & 0x01));
 					}
-					ctrlVal = 0;
-					for (k = 0; k < state->CH_Ctrl[i].size; k++)
-						ctrlVal += state->CH_Ctrl[i].val[k] * (1 << k);
 				} else
 					return -1;
 			}
@@ -3477,11 +3470,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
 							(u8)(state->MXL_Ctrl[i].bit[j]),
 							(u8)((value>>j) & 0x01));
 					}
-					ctrlVal = 0;
-					for (k = 0; k < state->MXL_Ctrl[i].size; k++)
-						ctrlVal += state->
-							MXL_Ctrl[i].val[k] *
-							(1 << k);
 				} else
 					return -1;
 			}
-- 
2.33.1

Re: [PATCH 20/20] media: mxl5005s: drop some dead code

[Nathan Chancellor]

On Wed, Nov 24, 2021 at 08:13:23PM +0100, Mauro Carvalho Chehab wrote:
> As reported by clang (with W=1), the ctrlVal var is never used.
> 
> Yet, there are even some loops to estimate it. As this is dead
> code, remove it. If ever needed, someone could revert this
> patch.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/20] at: https://lore.kernel.org/all/cover.1637781097.git.mchehab+huawei@kernel.org/
> 
>  drivers/media/tuners/mxl5005s.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/media/tuners/mxl5005s.c b/drivers/media/tuners/mxl5005s.c
> index f6e82a8e7d37..ab4c43df9d18 100644
> --- a/drivers/media/tuners/mxl5005s.c
> +++ b/drivers/media/tuners/mxl5005s.c
> @@ -3414,9 +3414,8 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
>  	u32 value, u16 controlGroup)
>  {
>  	struct mxl5005s_state *state = fe->tuner_priv;
> -	u16 i, j, k;
> +	u16 i, j;
>  	u32 highLimit;
> -	u32 ctrlVal;
>  
>  	if (controlGroup == 1) /* Initial Control */ {
>  
> @@ -3432,9 +3431,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
>  							(u8)(state->Init_Ctrl[i].bit[j]),
>  							(u8)((value>>j) & 0x01));
>  					}
> -					ctrlVal = 0;
> -					for (k = 0; k < state->Init_Ctrl[i].size; k++)
> -						ctrlVal += state->Init_Ctrl[i].val[k] * (1 << k);
>  				} else
>  					return -1;
>  			}
> @@ -3454,9 +3450,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
>  							(u8)(state->CH_Ctrl[i].bit[j]),
>  							(u8)((value>>j) & 0x01));
>  					}
> -					ctrlVal = 0;
> -					for (k = 0; k < state->CH_Ctrl[i].size; k++)
> -						ctrlVal += state->CH_Ctrl[i].val[k] * (1 << k);
>  				} else
>  					return -1;
>  			}
> @@ -3477,11 +3470,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum,
>  							(u8)(state->MXL_Ctrl[i].bit[j]),
>  							(u8)((value>>j) & 0x01));
>  					}
> -					ctrlVal = 0;
> -					for (k = 0; k < state->MXL_Ctrl[i].size; k++)
> -						ctrlVal += state->
> -							MXL_Ctrl[i].val[k] *
> -							(1 << k);
>  				} else
>  					return -1;
>  			}
> -- 
> 2.33.1
> 
> 

Re: [PATCH 00/20] Solve the remaining issues with clang and W=1 on media

[Arnd Bergmann]

On Wed, Nov 24, 2021 at 8:13 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Currently, using clang with W=1 and CONFIG_WERROR causes media to break.
> This is reported by builder.linuxtv.org.
>
> This series solve the remaining issues.

I looked through my randconfig tree and found one more that I had
initially not sent
as I wasn't sure whether we want to fix
-Wtautological-constant-out-of-range-compare
warnings. I'll send it now, as all warnings of that type in other
drivers have been addressed.

        Arnd

[PATCH v2 00/20] Solve the remaining issues with clang and W=1 on media

[Mauro Carvalho Chehab]

Currently, using clang with W=1 and CONFIG_WERROR causes media to break.
This is reported by builder.linuxtv.org.

This series solve the remaining issues.

v2:
  - Use __always_unused for the code we're keeping
  - Dropped a couple more unused functions
  - Added reviews got from the first submission and addressed some comments.

Mauro Carvalho Chehab (19):
  media: adv7842: get rid of two unused functions
  media: saa7134-go7007: get rid of to_state() function
  media: davinci: get rid of an unused function
  media: drxd: drop offset var from DownloadMicrocode()
  media: drxk: drop operation_mode from set_dvbt()
  media: m88ds3103: drop reg11 calculus from m88ds3103b_select_mclk()
  media: si21xx: report eventual errors at set_frontend
  media: solo6x10: mark unused functions as such
  media: si470x: fix printk warnings with clang
  media: radio-si476x: drop a container_of() abstraction macro
  media: lmedm04: don't ignore errors when setting a filter
  media: au0828-i2c: drop a duplicated function
  media: adv7604: mark unused functions as such
  media: adv7511: drop unused functions
  media: imx290: mark read reg function as __always_unused
  media: davinci: vpbe_osd: mark read reg function as __always_unused
  media: mtk-mdp: address a clang warning
  media: cobalt: drop an unused variable
  media: mxl5005s: drop some dead code

Robert Foss (1):
  media: camss: Remove unused static function

 drivers/media/dvb-frontends/drxd_hard.c       |  8 -------
 drivers/media/dvb-frontends/drxk_hard.c       | 23 ++++---------------
 drivers/media/dvb-frontends/m88ds3103.c       |  6 +----
 drivers/media/dvb-frontends/si21xx.c          |  7 +++---
 drivers/media/i2c/adv7511-v4l2.c              | 22 ------------------
 drivers/media/i2c/adv7604.c                   | 18 +++++++--------
 drivers/media/i2c/adv7842.c                   | 10 --------
 drivers/media/i2c/imx290.c                    |  2 +-
 drivers/media/pci/cobalt/cobalt-cpld.c        |  5 +---
 drivers/media/pci/saa7134/saa7134-go7007.c    |  7 +-----
 .../media/pci/solo6x10/solo6x10-v4l2-enc.c    | 12 +++++-----
 drivers/media/platform/davinci/vpbe_osd.c     |  2 +-
 drivers/media/platform/davinci/vpif_capture.c | 11 ---------
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c |  2 +-
 .../media/platform/qcom/camss/camss-vfe-170.c |  7 ------
 drivers/media/radio/radio-si476x.c            |  6 -----
 drivers/media/radio/si470x/radio-si470x-i2c.c |  4 ++--
 drivers/media/radio/si470x/radio-si470x-usb.c |  8 +++----
 drivers/media/tuners/mxl5005s.c               | 14 +----------
 drivers/media/usb/au0828/au0828-i2c.c         |  7 ------
 drivers/media/usb/dvb-usb-v2/lmedm04.c        |  3 +++
 21 files changed, 40 insertions(+), 144 deletions(-)

-- 
2.33.1

[PATCH v2 05/20] media: drxk: drop operation_mode from set_dvbt()

[Mauro Carvalho Chehab]

This var is set, but never used. So, drop it.

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

See [PATCH v2 00/20] at: https://lore.kernel.org/all/cover.1638179135.git.mchehab+huawei@kernel.org/

 drivers/media/dvb-frontends/drxk_hard.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-frontends/drxk_hard.c b/drivers/media/dvb-frontends/drxk_hard.c
index d7fc2595f15b..9430295a8175 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -3720,7 +3720,6 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 {
 	u16 cmd_result = 0;
 	u16 transmission_params = 0;
-	u16 operation_mode = 0;
 	u32 iqm_rc_rate_ofs = 0;
 	u32 bandwidth = 0;
 	u16 param1;
@@ -3759,10 +3758,8 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* mode */
 	switch (state->props.transmission_mode) {
 	case TRANSMISSION_MODE_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_MODE__M;
-		fallthrough;	/* try first guess DRX_FFTMODE_8K */
 	case TRANSMISSION_MODE_8K:
+	default:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_MODE_8K;
 		break;
 	case TRANSMISSION_MODE_2K:
@@ -3773,9 +3770,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* guard */
 	switch (state->props.guard_interval) {
 	default:
-	case GUARD_INTERVAL_AUTO:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_GUARD__M;
-		fallthrough;	/* try first guess DRX_GUARD_1DIV4 */
+	case GUARD_INTERVAL_AUTO: /* try first guess DRX_GUARD_1DIV4 */
 	case GUARD_INTERVAL_1_4:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_4;
 		break;
@@ -3794,11 +3789,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	switch (state->props.hierarchy) {
 	case HIERARCHY_AUTO:
 	case HIERARCHY_NONE:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_HIER__M;
-		/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
-		/* transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_NO; */
-		fallthrough;
+	default:	/* try first guess SC_RA_RAM_OP_PARAM_HIER_NO */
 	case HIERARCHY_1:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_HIER_A1;
 		break;
@@ -3814,9 +3805,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* modulation */
 	switch (state->props.modulation) {
 	case QAM_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_CONST__M;
-		fallthrough;	/* try first guess DRX_CONSTELLATION_QAM64 */
+	default:	/* try first guess DRX_CONSTELLATION_QAM64 */
 	case QAM_64:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_CONST_QAM64;
 		break;
@@ -3857,9 +3846,7 @@ static int set_dvbt(struct drxk_state *state, u16 intermediate_freqk_hz,
 	/* coderate */
 	switch (state->props.code_rate_HP) {
 	case FEC_AUTO:
-	default:
-		operation_mode |= OFDM_SC_RA_RAM_OP_AUTO_RATE__M;
-		fallthrough;	/* try first guess DRX_CODERATE_2DIV3 */
+	default:	/* try first guess DRX_CODERATE_2DIV3 */
 	case FEC_2_3:
 		transmission_params |= OFDM_SC_RA_RAM_OP_PARAM_RATE_2_3;
 		break;
-- 
2.33.1