* Re: [PATCH v2] media: i2c: alvium: Accelerated alvium_set_power
2025-09-09 11:22 [PATCH v2] media: i2c: alvium: Accelerated alvium_set_power Martin Hecht
@ 2025-09-10 2:31 ` kernel test robot
2025-09-10 15:09 ` Sakari Ailus
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2025-09-10 2:31 UTC (permalink / raw)
To: Martin Hecht, linux-media
Cc: oe-kbuild-all, sakari.ailus, michael.roeder, martin.hecht,
Martin Hecht, Tommaso Merciai, Mauro Carvalho Chehab,
linux-kernel
Hi Martin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on sailus-media-tree/master]
[also build test WARNING on linuxtv-media-pending/master media-tree/master linus/master v6.17-rc5 next-20250909]
[cannot apply to sailus-media-tree/streams]
[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/Martin-Hecht/media-i2c-alvium-Accelerated-alvium_set_power/20250909-193217
base: git://linuxtv.org/sailus/media_tree.git master
patch link: https://lore.kernel.org/r/20250909112252.2577949-1-mhecht73%40gmail.com
patch subject: [PATCH v2] media: i2c: alvium: Accelerated alvium_set_power
config: alpha-randconfig-r072-20250910 (https://download.01.org/0day-ci/archive/20250910/202509101013.wDfoHTNv-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250910/202509101013.wDfoHTNv-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/202509101013.wDfoHTNv-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/media/i2c/alvium-csi2.c: In function 'alvium_check':
>> drivers/media/i2c/alvium-csi2.c:2367:24: warning: unused variable 'dev' [-Wunused-variable]
2367 | struct device *dev = &alvium->i2c_client->dev;
| ^~~
vim +/dev +2367 drivers/media/i2c/alvium-csi2.c
2364
2365 static int alvium_check(struct alvium_dev *alvium, u64 *bcrm_major)
2366 {
> 2367 struct device *dev = &alvium->i2c_client->dev;
2368 int ret = 0;
2369
2370 ret = alvium_read(alvium, REG_BCRM_MAJOR_VERSION_R, bcrm_major, NULL);
2371
2372 if (ret)
2373 return ret;
2374
2375 if (*bcrm_major != 0)
2376 return 0;
2377
2378 return -ENODEV;
2379 }
2380
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] media: i2c: alvium: Accelerated alvium_set_power
2025-09-09 11:22 [PATCH v2] media: i2c: alvium: Accelerated alvium_set_power Martin Hecht
2025-09-10 2:31 ` kernel test robot
@ 2025-09-10 15:09 ` Sakari Ailus
1 sibling, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2025-09-10 15:09 UTC (permalink / raw)
To: Martin Hecht
Cc: linux-media, michael.roeder, martin.hecht, Tommaso Merciai,
Mauro Carvalho Chehab, linux-kernel
Hi Martin,
On Tue, Sep 09, 2025 at 01:22:51PM +0200, Martin Hecht wrote:
> Now alvium_set_power tests if Alvium is up and running already
> instead of waiting for the period of a full reboot. This safes
> about 5-7 seconds delay for each connected camera what is already
> booted especially when using multiple Alvium cameras or using
> camera arrays.
> The new function alvium_check is used by read_poll_timeout to check
> whether a camera is connected on I2C and if it responds already.
>
> Signed-off-by: Martin Hecht <mhecht73@gmail.com>
> ---
> v2:
> - added alvium_check to be used by read_poll_timeout as
> suggested by Sakari
> ---
> drivers/media/i2c/alvium-csi2.c | 32 +++++++++++++++++++++++++-------
> 1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> index 5c1bab574394..c63af96d3b31 100644
> --- a/drivers/media/i2c/alvium-csi2.c
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -443,10 +443,8 @@ static int alvium_is_alive(struct alvium_dev *alvium)
>
> alvium_read(alvium, REG_BCRM_MINOR_VERSION_R, &bcrm, &ret);
> alvium_read(alvium, REG_BCRM_HEARTBEAT_RW, &hbeat, &ret);
> - if (ret)
> - return ret;
>
> - return hbeat;
> + return ret;
> }
>
> static void alvium_print_avail_mipi_fmt(struct alvium_dev *alvium)
> @@ -2364,8 +2362,25 @@ static int alvium_get_dt_data(struct alvium_dev *alvium)
> return -EINVAL;
> }
>
> +static int alvium_check(struct alvium_dev *alvium, u64 *bcrm_major)
> +{
> + struct device *dev = &alvium->i2c_client->dev;
> + int ret = 0;
No need to assign ret here.
> +
> + ret = alvium_read(alvium, REG_BCRM_MAJOR_VERSION_R, bcrm_major, NULL);
> +
No need for an empty line here.
But see below...
> + if (ret)
> + return ret;
> +
> + if (*bcrm_major != 0)
> + return 0;
> +
> + return -ENODEV;
> +}
> +
> static int alvium_set_power(struct alvium_dev *alvium, bool on)
> {
> + u64 bcrm_major = 0;
> int ret;
>
> if (!on)
> @@ -2375,9 +2390,12 @@ static int alvium_set_power(struct alvium_dev *alvium, bool on)
> if (ret)
> return ret;
>
> - /* alvium boot time 7s */
> - msleep(7000);
> - return 0;
> + /* alvium boot time is up to 7.5s but test if its available already */
> + read_poll_timeout(alvium_check, bcrm_major, (bcrm_major == 0),
> + 250000, 7500000, false,
> + alvium, &bcrm_major);
I presume bcrm_major needs to be non-zero to proceed rather than zero?
I think you could also do:
read_poll_timeout(alvium_read, ret, !ret && brcm_major, 250000, 7500000,
false, alvium, REG_BCRM_MAJOR_VERSION_R, bcrm_major,
NULL);
return ret ?: brcm_major ? 0 : -ENODEV;
> +
> + return ret;
> }
>
> static int alvium_runtime_resume(struct device *dev)
> @@ -2442,7 +2460,7 @@ static int alvium_probe(struct i2c_client *client)
> if (ret)
> goto err_powerdown;
>
> - if (!alvium_is_alive(alvium)) {
> + if (alvium_is_alive(alvium)) {
If you prefer to change this, then I'd assign the return value to ret, as
returned by alvium_read() and use it as the error code here, too. But this
should be a separate patch.
> ret = -ENODEV;
> dev_err_probe(dev, ret, "Device detection failed\n");
> goto err_powerdown;
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 3+ messages in thread