* Re: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
2023-10-29 22:07 [PATCH] Media: omap4iss: Enable RSZB and update resizer control Nancy Nyambura
@ 2023-10-29 23:23 ` Laurent Pinchart
2023-10-30 3:19 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2023-10-29 23:23 UTC (permalink / raw)
To: Nancy Nyambura
Cc: gagallo7+outreachy, nicydaniels, outreachy, Mauro Carvalho Chehab,
Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel
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.
> }
>
> /* -----------------------------------------------------------------------------
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
2023-10-29 22:07 [PATCH] Media: omap4iss: Enable RSZB and update resizer control Nancy Nyambura
2023-10-29 23:23 ` Laurent Pinchart
@ 2023-10-30 3:19 ` kernel test robot
2023-10-30 6:31 ` Julia Lawall
2023-10-30 7:32 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-10-30 3:19 UTC (permalink / raw)
To: Nancy Nyambura, gagallo7+outreachy
Cc: oe-kbuild-all, nicydaniels, outreachy, Nancy Nyambura,
Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
Greg Kroah-Hartman, linux-staging, linux-kernel
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
2023-10-29 22:07 [PATCH] Media: omap4iss: Enable RSZB and update resizer control Nancy Nyambura
2023-10-29 23:23 ` Laurent Pinchart
2023-10-30 3:19 ` kernel test robot
@ 2023-10-30 6:31 ` Julia Lawall
2023-10-30 7:32 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2023-10-30 6:31 UTC (permalink / raw)
To: Nancy Nyambura
Cc: gagallo7+outreachy, nicydaniels, outreachy, Laurent Pinchart,
Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media,
linux-staging, linux-kernel
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
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Media: omap4iss: Enable RSZB and update resizer control
2023-10-29 22:07 [PATCH] Media: omap4iss: Enable RSZB and update resizer control Nancy Nyambura
` (2 preceding siblings ...)
2023-10-30 6:31 ` Julia Lawall
@ 2023-10-30 7:32 ` kernel test robot
3 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-10-30 7:32 UTC (permalink / raw)
To: Nancy Nyambura, gagallo7+outreachy
Cc: oe-kbuild-all, nicydaniels, outreachy, Nancy Nyambura,
Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
Greg Kroah-Hartman, linux-staging, linux-kernel
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread