diff mbox series

Media: omap4iss: Enable RSZB and update resizer control

Message ID 20231029220710.47063-1-nicymimz@gmail.com (mailing list archive)
State New, archived
Headers show
Series Media: omap4iss: Enable RSZB and update resizer control | expand

Commit Message

Nancy Nyambura Oct. 29, 2023, 10:07 p.m. UTC
Enable RSZB functionality in the OMAP4 ISS driver. This change sets the RSZB system configuration register to enable the RSZB module. Additionally, it updates the resizer control by setting the RSZ_EN_EN flag as required. This change enhances the driver's capabilities and prepares it for future developments.

Signed-off-by: Nancy Nyambura <nicymimz@gmail.com>
---
 drivers/staging/media/omap4iss/iss_resizer.c | 34 +++++++++++---------
 1 file changed, 19 insertions(+), 15 deletions(-)

Comments

Laurent Pinchart Oct. 29, 2023, 11:23 p.m. UTC | #1
Hello Nancy,

On Mon, Oct 30, 2023 at 01:07:09AM +0300, Nancy Nyambura wrote:
> Enable RSZB functionality in the OMAP4 ISS driver. This change sets the RSZB system configuration register to enable the RSZB module. Additionally, it updates the resizer control by setting the RSZ_EN_EN flag as required. This change enhances the driver's capabilities and prepares it for future developments.

You haven't run this through checkpatch, have you ?

> Signed-off-by: Nancy Nyambura <nicymimz@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss_resizer.c | 34 +++++++++++---------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss_resizer.c b/drivers/staging/media/omap4iss/iss_resizer.c
> index a5f8f9f1ab16..23089eeaf448 100644
> --- a/drivers/staging/media/omap4iss/iss_resizer.c
> +++ b/drivers/staging/media/omap4iss/iss_resizer.c
> @@ -7,17 +7,17 @@
>   * Author: Sergio Aguirre <sergio.a.aguirre@gmail.com>
>   */
>  
> -#include <linux/module.h>
> -#include <linux/uaccess.h>
> -#include <linux/delay.h>
> -#include <linux/device.h>
> -#include <linux/dma-mapping.h>
> -#include <linux/mm.h>
> -#include <linux/sched.h>
> -
> -#include "iss.h"
> -#include "iss_regs.h"
> -#include "iss_resizer.h"
> + #include <linux/module.h>
> + #include <linux/uaccess.h>
> + #include <linux/delay.h>
> + #include <linux/device.h>
> + #include <linux/dma-mapping.h>
> + #include <linux/mm.h>
> + #include <linux/sched.h>
> +
> + #include "iss.h"
> + #include "iss_regs.h"
> + #include "iss_resizer.h"

Or even read the patch before sending it out ?

>  
>  static const unsigned int resizer_fmts[] = {
>  	MEDIA_BUS_FMT_UYVY8_1X16,
> @@ -30,11 +30,11 @@ static const unsigned int resizer_fmts[] = {
>   *
>   * Also prints other debug information stored in the RESIZER module.
>   */
> -#define RSZ_PRINT_REGISTER(iss, name)\
> + #define RSZ_PRINT_REGISTER(iss, name)\
>  	dev_dbg(iss->dev, "###RSZ " #name "=0x%08x\n", \
>  		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RSZ_##name))
>  
> -#define RZA_PRINT_REGISTER(iss, name)\
> + #define RZA_PRINT_REGISTER(iss, name)\
>  	dev_dbg(iss->dev, "###RZA " #name "=0x%08x\n", \
>  		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_##name))
>  
> @@ -116,8 +116,12 @@ static void resizer_enable(struct iss_resizer_device *resizer, u8 enable)
>  		       RSZ_SRC_EN_SRC_EN, enable ? RSZ_SRC_EN_SRC_EN : 0);
>  
>  	/* TODO: Enable RSZB */
> -	iss_reg_update(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_EN, RSZ_EN_EN,
> -		       enable ? RSZ_EN_EN : 0);
> +	u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
> +		       	+ RZ_SYSCONFIG);
> +	reg_value |= RSZ_SYSCONFIG_RSZB_CLK_EN;
> +	iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER, 
> +			+ RSZ_SYSCONFIG);
> +

This doesn't even compile.

Has this all been generated by chatgpt by any chance ? It doesn't look
like whoever wrote this understand what they were doing.

>  }
>  
>  /* -----------------------------------------------------------------------------
kernel test robot Oct. 30, 2023, 3:19 a.m. UTC | #2
Hi Nancy,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v6.6-rc7 next-20231027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nancy-Nyambura/Media-omap4iss-Enable-RSZB-and-update-resizer-control/20231030-060944
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20231029220710.47063-1-nicymimz%40gmail.com
patch subject: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20231030/202310301031.IZuhUjMc-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301031.IZuhUjMc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310301031.IZuhUjMc-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/staging/media/omap4iss/iss_resizer.c: In function 'resizer_enable':
   drivers/staging/media/omap4iss/iss_resizer.c:119:37: error: 'struct iss_device' has no member named 'base_addr'
     119 |         u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                                     ^~
   drivers/staging/media/omap4iss/iss_resizer.c:120:27: error: 'RZ_SYSCONFIG' undeclared (first use in this function); did you mean 'RSZ_SYSCONFIG'?
     120 |                         + RZ_SYSCONFIG);
         |                           ^~~~~~~~~~~~
         |                           RSZ_SYSCONFIG
   drivers/staging/media/omap4iss/iss_resizer.c:120:27: note: each undeclared identifier is reported only once for each function it appears in
   In file included from include/linux/scatterlist.h:9,
                    from include/linux/dma-mapping.h:11,
                    from drivers/staging/media/omap4iss/iss_resizer.c:14:
>> arch/sh/include/asm/io.h:129:18: error: too many arguments to function 'ioread32'
     129 | #define ioread32 ioread32
         |                  ^~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:119:25: note: in expansion of macro 'ioread32'
     119 |         u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                         ^~~~~~~~
   In file included from arch/sh/include/asm/io.h:22:
   include/asm-generic/iomap.h:32:21: note: declared here
      32 | extern unsigned int ioread32(const void __iomem *);
         |                     ^~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:122:33: error: 'struct iss_device' has no member named 'base_addr'
     122 |         iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                                 ^~
>> arch/sh/include/asm/io.h:135:19: error: too many arguments to function 'iowrite32'
     135 | #define iowrite32 iowrite32
         |                   ^~~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:122:9: note: in expansion of macro 'iowrite32'
     122 |         iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |         ^~~~~~~~~
   include/asm-generic/iomap.h:53:13: note: declared here
      53 | extern void iowrite32(u32, void __iomem *);
         |             ^~~~~~~~~


vim +/ioread32 +129 arch/sh/include/asm/io.h

b94692e84dccf1 Baoquan He 2023-07-06  125  
b94692e84dccf1 Baoquan He 2023-07-06  126  #define ioread8 ioread8
b94692e84dccf1 Baoquan He 2023-07-06  127  #define ioread16 ioread16
b94692e84dccf1 Baoquan He 2023-07-06  128  #define ioread16be ioread16be
b94692e84dccf1 Baoquan He 2023-07-06 @129  #define ioread32 ioread32
b94692e84dccf1 Baoquan He 2023-07-06  130  #define ioread32be ioread32be
b94692e84dccf1 Baoquan He 2023-07-06  131  
b94692e84dccf1 Baoquan He 2023-07-06  132  #define iowrite8 iowrite8
b94692e84dccf1 Baoquan He 2023-07-06  133  #define iowrite16 iowrite16
b94692e84dccf1 Baoquan He 2023-07-06  134  #define iowrite16be iowrite16be
b94692e84dccf1 Baoquan He 2023-07-06 @135  #define iowrite32 iowrite32
b94692e84dccf1 Baoquan He 2023-07-06  136  #define iowrite32be iowrite32be
b94692e84dccf1 Baoquan He 2023-07-06  137
Julia Lawall Oct. 30, 2023, 6:31 a.m. UTC | #3
On Mon, 30 Oct 2023, Nancy Nyambura wrote:

> Enable RSZB functionality in the OMAP4 ISS driver. This change sets the RSZB system configuration register to enable the RSZB module. Additionally, it updates the resizer control by setting the RSZ_EN_EN flag as required. This change enhances the driver's capabilities and prepares it for future developments.

Could you explain more about your changes?  What information led you to
make these changes.  The current messages says what is done, but not so
much about why.

Also, the log message should be limited to around 70 characters per line,
so that it looks nice in the git history after it has been indented.

julia


>
> Signed-off-by: Nancy Nyambura <nicymimz@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss_resizer.c | 34 +++++++++++---------
>  1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss_resizer.c b/drivers/staging/media/omap4iss/iss_resizer.c
> index a5f8f9f1ab16..23089eeaf448 100644
> --- a/drivers/staging/media/omap4iss/iss_resizer.c
> +++ b/drivers/staging/media/omap4iss/iss_resizer.c
> @@ -7,17 +7,17 @@
>   * Author: Sergio Aguirre <sergio.a.aguirre@gmail.com>
>   */
>
> -#include <linux/module.h>
> -#include <linux/uaccess.h>
> -#include <linux/delay.h>
> -#include <linux/device.h>
> -#include <linux/dma-mapping.h>
> -#include <linux/mm.h>
> -#include <linux/sched.h>
> -
> -#include "iss.h"
> -#include "iss_regs.h"
> -#include "iss_resizer.h"
> + #include <linux/module.h>
> + #include <linux/uaccess.h>
> + #include <linux/delay.h>
> + #include <linux/device.h>
> + #include <linux/dma-mapping.h>
> + #include <linux/mm.h>
> + #include <linux/sched.h>
> +
> + #include "iss.h"
> + #include "iss_regs.h"
> + #include "iss_resizer.h"
>
>  static const unsigned int resizer_fmts[] = {
>  	MEDIA_BUS_FMT_UYVY8_1X16,
> @@ -30,11 +30,11 @@ static const unsigned int resizer_fmts[] = {
>   *
>   * Also prints other debug information stored in the RESIZER module.
>   */
> -#define RSZ_PRINT_REGISTER(iss, name)\
> + #define RSZ_PRINT_REGISTER(iss, name)\
>  	dev_dbg(iss->dev, "###RSZ " #name "=0x%08x\n", \
>  		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RSZ_##name))
>
> -#define RZA_PRINT_REGISTER(iss, name)\
> + #define RZA_PRINT_REGISTER(iss, name)\
>  	dev_dbg(iss->dev, "###RZA " #name "=0x%08x\n", \
>  		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_##name))
>
> @@ -116,8 +116,12 @@ static void resizer_enable(struct iss_resizer_device *resizer, u8 enable)
>  		       RSZ_SRC_EN_SRC_EN, enable ? RSZ_SRC_EN_SRC_EN : 0);
>
>  	/* TODO: Enable RSZB */
> -	iss_reg_update(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_EN, RSZ_EN_EN,
> -		       enable ? RSZ_EN_EN : 0);
> +	u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
> +		       	+ RZ_SYSCONFIG);
> +	reg_value |= RSZ_SYSCONFIG_RSZB_CLK_EN;
> +	iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
> +			+ RSZ_SYSCONFIG);
> +
>  }
>
>  /* -----------------------------------------------------------------------------
> --
> 2.40.1
>
>
>
kernel test robot Oct. 30, 2023, 7:32 a.m. UTC | #4
Hi Nancy,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v6.6-rc7 next-20231027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nancy-Nyambura/Media-omap4iss-Enable-RSZB-and-update-resizer-control/20231030-060944
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20231029220710.47063-1-nicymimz%40gmail.com
patch subject: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231030/202310301558.abL6OUgF-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231030/202310301558.abL6OUgF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310301558.abL6OUgF-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/staging/media/omap4iss/iss_resizer.c: In function 'resizer_enable':
   drivers/staging/media/omap4iss/iss_resizer.c:119:37: error: 'struct iss_device' has no member named 'base_addr'
     119 |         u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                                     ^~
   drivers/staging/media/omap4iss/iss_resizer.c:120:27: error: 'RZ_SYSCONFIG' undeclared (first use in this function); did you mean 'RSZ_SYSCONFIG'?
     120 |                         + RZ_SYSCONFIG);
         |                           ^~~~~~~~~~~~
         |                           RSZ_SYSCONFIG
   drivers/staging/media/omap4iss/iss_resizer.c:120:27: note: each undeclared identifier is reported only once for each function it appears in
   In file included from include/linux/scatterlist.h:9,
                    from include/linux/dma-mapping.h:11,
                    from drivers/staging/media/omap4iss/iss_resizer.c:14:
>> arch/alpha/include/asm/io.h:433:18: error: too many arguments to function 'ioread32'
     433 | #define ioread32 ioread32
         |                  ^~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:119:25: note: in expansion of macro 'ioread32'
     119 |         u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                         ^~~~~~~~
   In file included from arch/alpha/include/asm/io.h:15:
   include/asm-generic/iomap.h:32:21: note: declared here
      32 | extern unsigned int ioread32(const void __iomem *);
         |                     ^~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:122:33: error: 'struct iss_device' has no member named 'base_addr'
     122 |         iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |                                 ^~
>> arch/alpha/include/asm/io.h:435:19: error: too many arguments to function 'iowrite32'
     435 | #define iowrite32 iowrite32
         |                   ^~~~~~~~~
   drivers/staging/media/omap4iss/iss_resizer.c:122:9: note: in expansion of macro 'iowrite32'
     122 |         iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
         |         ^~~~~~~~~
   include/asm-generic/iomap.h:53:13: note: declared here
      53 | extern void iowrite32(u32, void __iomem *);
         |             ^~~~~~~~~


vim +/ioread32 +433 arch/alpha/include/asm/io.h

^1da177e4c3f41 include/asm-alpha/io.h      Linus Torvalds 2005-04-16  432  
7e772dad991399 arch/alpha/include/asm/io.h Linus Walleij  2022-09-06 @433  #define ioread32 ioread32
e19d4ebc536dad arch/alpha/include/asm/io.h Arnd Bergmann  2022-10-03  434  #define ioread64 ioread64
7e772dad991399 arch/alpha/include/asm/io.h Linus Walleij  2022-09-06 @435  #define iowrite32 iowrite32
e19d4ebc536dad arch/alpha/include/asm/io.h Arnd Bergmann  2022-10-03  436  #define iowrite64 iowrite64
7e772dad991399 arch/alpha/include/asm/io.h Linus Walleij  2022-09-06  437
diff mbox series

Patch

diff --git a/drivers/staging/media/omap4iss/iss_resizer.c b/drivers/staging/media/omap4iss/iss_resizer.c
index a5f8f9f1ab16..23089eeaf448 100644
--- a/drivers/staging/media/omap4iss/iss_resizer.c
+++ b/drivers/staging/media/omap4iss/iss_resizer.c
@@ -7,17 +7,17 @@ 
  * Author: Sergio Aguirre <sergio.a.aguirre@gmail.com>
  */
 
-#include <linux/module.h>
-#include <linux/uaccess.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/dma-mapping.h>
-#include <linux/mm.h>
-#include <linux/sched.h>
-
-#include "iss.h"
-#include "iss_regs.h"
-#include "iss_resizer.h"
+ #include <linux/module.h>
+ #include <linux/uaccess.h>
+ #include <linux/delay.h>
+ #include <linux/device.h>
+ #include <linux/dma-mapping.h>
+ #include <linux/mm.h>
+ #include <linux/sched.h>
+
+ #include "iss.h"
+ #include "iss_regs.h"
+ #include "iss_resizer.h"
 
 static const unsigned int resizer_fmts[] = {
 	MEDIA_BUS_FMT_UYVY8_1X16,
@@ -30,11 +30,11 @@  static const unsigned int resizer_fmts[] = {
  *
  * Also prints other debug information stored in the RESIZER module.
  */
-#define RSZ_PRINT_REGISTER(iss, name)\
+ #define RSZ_PRINT_REGISTER(iss, name)\
 	dev_dbg(iss->dev, "###RSZ " #name "=0x%08x\n", \
 		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RSZ_##name))
 
-#define RZA_PRINT_REGISTER(iss, name)\
+ #define RZA_PRINT_REGISTER(iss, name)\
 	dev_dbg(iss->dev, "###RZA " #name "=0x%08x\n", \
 		iss_reg_read(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_##name))
 
@@ -116,8 +116,12 @@  static void resizer_enable(struct iss_resizer_device *resizer, u8 enable)
 		       RSZ_SRC_EN_SRC_EN, enable ? RSZ_SRC_EN_SRC_EN : 0);
 
 	/* TODO: Enable RSZB */
-	iss_reg_update(iss, OMAP4_ISS_MEM_ISP_RESIZER, RZA_EN, RSZ_EN_EN,
-		       enable ? RSZ_EN_EN : 0);
+	u32 reg_value = ioread32(iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER,
+		       	+ RZ_SYSCONFIG);
+	reg_value |= RSZ_SYSCONFIG_RSZB_CLK_EN;
+	iowrite32(reg_value, iss->base_addr + OMAP4_ISS_MEM_ISP_RESIZER, 
+			+ RSZ_SYSCONFIG);
+
 }
 
 /* -----------------------------------------------------------------------------