* Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
2020-10-13 15:36 ` [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper Sakari Ailus
@ 2020-10-13 17:16 ` kernel test robot
2020-10-14 8:30 ` [PATCH v2 " Sakari Ailus
2020-10-15 22:47 ` kernel test robot
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-10-13 17:16 UTC (permalink / raw)
To: Sakari Ailus, linux-media; +Cc: kbuild-all, laurent.pinchart
[-- Attachment #1: Type: text/plain, Size: 4474 bytes --]
Hi Sakari,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201013]
[cannot apply to v5.9]
[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]
url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base: git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/pci.h:37,
from drivers/media/pci/intel/ipu3/ipu3-cio2.c:18:
drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_csi2_calc_timing':
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 's64' {aka 'long long int'} [-Wformat=]
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:3: note: in expansion of macro 'dev_err'
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ^~~~~~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:25: note: format string is defined here
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ~~^
| |
| long int
| %lld
drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
301 | int r;
| ^
vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c
293
294 /* Calculate the the delay value for termination enable of clock lane HS Rx */
295 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
296 struct cio2_csi2_timing *timing,
297 unsigned int bpp, unsigned int lanes)
298 {
299 struct device *dev = &cio2->pci_dev->dev;
300 s64 freq;
301 int r;
302
303 if (!q->sensor)
304 return -ENODEV;
305
306 freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
307 if (freq < 0) {
> 308 dev_err(dev, "error %ld, invalid link_freq\n", freq);
309 return freq;
310 }
311
312 timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
313 CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
314 freq,
315 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
316 timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
317 CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
318 freq,
319 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
320 timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
321 CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
322 freq,
323 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
324 timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
325 CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
326 freq,
327 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
328
329 dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
330 dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
331 dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
332 dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
333
334 return 0;
335 };
336
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74757 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
@ 2020-10-13 17:16 ` kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-10-13 17:16 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4577 bytes --]
Hi Sakari,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201013]
[cannot apply to v5.9]
[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]
url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base: git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/pci.h:37,
from drivers/media/pci/intel/ipu3/ipu3-cio2.c:18:
drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_csi2_calc_timing':
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 's64' {aka 'long long int'} [-Wformat=]
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:3: note: in expansion of macro 'dev_err'
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ^~~~~~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:25: note: format string is defined here
308 | dev_err(dev, "error %ld, invalid link_freq\n", freq);
| ~~^
| |
| long int
| %lld
drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
301 | int r;
| ^
vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c
293
294 /* Calculate the the delay value for termination enable of clock lane HS Rx */
295 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
296 struct cio2_csi2_timing *timing,
297 unsigned int bpp, unsigned int lanes)
298 {
299 struct device *dev = &cio2->pci_dev->dev;
300 s64 freq;
301 int r;
302
303 if (!q->sensor)
304 return -ENODEV;
305
306 freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
307 if (freq < 0) {
> 308 dev_err(dev, "error %ld, invalid link_freq\n", freq);
309 return freq;
310 }
311
312 timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
313 CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
314 freq,
315 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
316 timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
317 CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
318 freq,
319 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
320 timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
321 CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
322 freq,
323 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
324 timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
325 CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
326 freq,
327 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
328
329 dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
330 dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
331 dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
332 dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
333
334 return 0;
335 };
336
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 74757 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
2020-10-13 15:36 ` [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper Sakari Ailus
2020-10-13 17:16 ` kernel test robot
@ 2020-10-14 8:30 ` Sakari Ailus
2021-02-15 0:12 ` Laurent Pinchart
2020-10-15 22:47 ` kernel test robot
2 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2020-10-14 8:30 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart
Use v4l2_get_link_freq helper and add support for sensor drivers
implementing only V4L2_CID_PIXEL_RATE.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1:
- Use %lld for printing long long int
- Remove r (unused variable)
drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index c557d189200b..d060cfe473d8 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
u32 mbus_code;
u32 fourcc;
u8 mipicode;
+ u8 bpp;
};
/*
@@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
.mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
.fourcc = V4L2_PIX_FMT_IPU3_SGRBG10,
.mipicode = 0x2b,
+ .bpp = 10,
}, {
.mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
.fourcc = V4L2_PIX_FMT_IPU3_SGBRG10,
.mipicode = 0x2b,
+ .bpp = 10,
}, {
.mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
.fourcc = V4L2_PIX_FMT_IPU3_SBGGR10,
.mipicode = 0x2b,
+ .bpp = 10,
}, {
.mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
.fourcc = V4L2_PIX_FMT_IPU3_SRGGB10,
.mipicode = 0x2b,
+ .bpp = 10,
},
};
@@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
/* Calculate the the delay value for termination enable of clock lane HS Rx */
static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
- struct cio2_csi2_timing *timing)
+ struct cio2_csi2_timing *timing,
+ unsigned int bpp, unsigned int lanes)
{
struct device *dev = &cio2->pci_dev->dev;
- struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
- struct v4l2_ctrl *link_freq;
s64 freq;
- int r;
if (!q->sensor)
return -ENODEV;
- link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
- if (!link_freq) {
- dev_err(dev, "failed to find LINK_FREQ\n");
- return -EPIPE;
- }
-
- qm.index = v4l2_ctrl_g_ctrl(link_freq);
- r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
- if (r) {
- dev_err(dev, "failed to get menu item\n");
- return r;
- }
-
- if (!qm.value) {
- dev_err(dev, "error invalid link_freq\n");
- return -EINVAL;
+ freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
+ if (freq < 0) {
+ dev_err(dev, "error %lld, invalid link_freq\n", freq);
+ return freq;
}
- freq = qm.value;
timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
@@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
lanes = q->csi2.lanes;
- r = cio2_csi2_calc_timing(cio2, q, &timing);
+ r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
if (r)
return r;
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
2020-10-14 8:30 ` [PATCH v2 " Sakari Ailus
@ 2021-02-15 0:12 ` Laurent Pinchart
2021-02-15 7:54 ` Sakari Ailus
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2021-02-15 0:12 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media
Hi Sakari,
Replying to an old thread.
On Wed, Oct 14, 2020 at 11:30:15AM +0300, Sakari Ailus wrote:
> Use v4l2_get_link_freq helper and add support for sensor drivers
> implementing only V4L2_CID_PIXEL_RATE.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> since v1:
>
> - Use %lld for printing long long int
>
> - Remove r (unused variable)
>
> drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
> 1 file changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index c557d189200b..d060cfe473d8 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
> u32 mbus_code;
> u32 fourcc;
> u8 mipicode;
> + u8 bpp;
> };
>
> /*
> @@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
> .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> .fourcc = V4L2_PIX_FMT_IPU3_SGRBG10,
> .mipicode = 0x2b,
> + .bpp = 10,
> }, {
> .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> .fourcc = V4L2_PIX_FMT_IPU3_SGBRG10,
> .mipicode = 0x2b,
> + .bpp = 10,
> }, {
> .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> .fourcc = V4L2_PIX_FMT_IPU3_SBGGR10,
> .mipicode = 0x2b,
> + .bpp = 10,
> }, {
> .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> .fourcc = V4L2_PIX_FMT_IPU3_SRGGB10,
> .mipicode = 0x2b,
> + .bpp = 10,
> },
> };
>
> @@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
>
> /* Calculate the the delay value for termination enable of clock lane HS Rx */
> static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
> - struct cio2_csi2_timing *timing)
> + struct cio2_csi2_timing *timing,
> + unsigned int bpp, unsigned int lanes)
> {
> struct device *dev = &cio2->pci_dev->dev;
> - struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
> - struct v4l2_ctrl *link_freq;
> s64 freq;
> - int r;
>
> if (!q->sensor)
> return -ENODEV;
>
> - link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
> - if (!link_freq) {
> - dev_err(dev, "failed to find LINK_FREQ\n");
> - return -EPIPE;
> - }
> -
> - qm.index = v4l2_ctrl_g_ctrl(link_freq);
> - r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
> - if (r) {
> - dev_err(dev, "failed to get menu item\n");
> - return r;
> - }
> -
> - if (!qm.value) {
> - dev_err(dev, "error invalid link_freq\n");
> - return -EINVAL;
> + freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
Shouldn't this use lanes * 2 ?
> + if (freq < 0) {
> + dev_err(dev, "error %lld, invalid link_freq\n", freq);
> + return freq;
> }
> - freq = qm.value;
>
> timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
> CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
> @@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
>
> lanes = q->csi2.lanes;
>
> - r = cio2_csi2_calc_timing(cio2, q, &timing);
> + r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
> if (r)
> return r;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
2021-02-15 0:12 ` Laurent Pinchart
@ 2021-02-15 7:54 ` Sakari Ailus
0 siblings, 0 replies; 10+ messages in thread
From: Sakari Ailus @ 2021-02-15 7:54 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media
Hi Laurent,
On Mon, Feb 15, 2021 at 02:12:23AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> Replying to an old thread.
>
> On Wed, Oct 14, 2020 at 11:30:15AM +0300, Sakari Ailus wrote:
> > Use v4l2_get_link_freq helper and add support for sensor drivers
> > implementing only V4L2_CID_PIXEL_RATE.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > since v1:
> >
> > - Use %lld for printing long long int
> >
> > - Remove r (unused variable)
> >
> > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 34 +++++++++---------------
> > 1 file changed, 12 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > index c557d189200b..d060cfe473d8 100644
> > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> > @@ -33,6 +33,7 @@ struct ipu3_cio2_fmt {
> > u32 mbus_code;
> > u32 fourcc;
> > u8 mipicode;
> > + u8 bpp;
> > };
> >
> > /*
> > @@ -46,18 +47,22 @@ static const struct ipu3_cio2_fmt formats[] = {
> > .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > .fourcc = V4L2_PIX_FMT_IPU3_SGRBG10,
> > .mipicode = 0x2b,
> > + .bpp = 10,
> > }, {
> > .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> > .fourcc = V4L2_PIX_FMT_IPU3_SGBRG10,
> > .mipicode = 0x2b,
> > + .bpp = 10,
> > }, {
> > .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> > .fourcc = V4L2_PIX_FMT_IPU3_SBGGR10,
> > .mipicode = 0x2b,
> > + .bpp = 10,
> > }, {
> > .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> > .fourcc = V4L2_PIX_FMT_IPU3_SRGGB10,
> > .mipicode = 0x2b,
> > + .bpp = 10,
> > },
> > };
> >
> > @@ -288,35 +293,20 @@ static s32 cio2_rx_timing(s32 a, s32 b, s64 freq, int def)
> >
> > /* Calculate the the delay value for termination enable of clock lane HS Rx */
> > static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
> > - struct cio2_csi2_timing *timing)
> > + struct cio2_csi2_timing *timing,
> > + unsigned int bpp, unsigned int lanes)
> > {
> > struct device *dev = &cio2->pci_dev->dev;
> > - struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
> > - struct v4l2_ctrl *link_freq;
> > s64 freq;
> > - int r;
> >
> > if (!q->sensor)
> > return -ENODEV;
> >
> > - link_freq = v4l2_ctrl_find(q->sensor->ctrl_handler, V4L2_CID_LINK_FREQ);
> > - if (!link_freq) {
> > - dev_err(dev, "failed to find LINK_FREQ\n");
> > - return -EPIPE;
> > - }
> > -
> > - qm.index = v4l2_ctrl_g_ctrl(link_freq);
> > - r = v4l2_querymenu(q->sensor->ctrl_handler, &qm);
> > - if (r) {
> > - dev_err(dev, "failed to get menu item\n");
> > - return r;
> > - }
> > -
> > - if (!qm.value) {
> > - dev_err(dev, "error invalid link_freq\n");
> > - return -EINVAL;
> > + freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
>
> Shouldn't this use lanes * 2 ?
Yes, it should. Good catch!
I'll send a patch...
>
> > + if (freq < 0) {
> > + dev_err(dev, "error %lld, invalid link_freq\n", freq);
> > + return freq;
> > }
> > - freq = qm.value;
> >
> > timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
> > CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
> > @@ -364,7 +354,7 @@ static int cio2_hw_init(struct cio2_device *cio2, struct cio2_queue *q)
> >
> > lanes = q->csi2.lanes;
> >
> > - r = cio2_csi2_calc_timing(cio2, q, &timing);
> > + r = cio2_csi2_calc_timing(cio2, q, &timing, fmt->bpp, lanes);
> > if (r)
> > return r;
> >
>
> --
> Regards,
>
> Laurent Pinchart
--
Sakari Ailus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
2020-10-13 15:36 ` [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper Sakari Ailus
@ 2020-10-15 22:47 ` kernel test robot
2020-10-14 8:30 ` [PATCH v2 " Sakari Ailus
2020-10-15 22:47 ` kernel test robot
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-10-15 22:47 UTC (permalink / raw)
To: Sakari Ailus, linux-media; +Cc: kbuild-all, clang-built-linux, laurent.pinchart
[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]
Hi Sakari,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201015]
[cannot apply to v5.9]
[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]
url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a012-20201015 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:50: warning: format specifies type 'long' but the argument has type 's64' (aka 'long long') [-Wformat]
dev_err(dev, "error %ld, invalid link_freq\n", freq);
~~~ ^~~~
%lld
include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
_dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
int r;
^
2 warnings generated.
vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c
293
294 /* Calculate the the delay value for termination enable of clock lane HS Rx */
295 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
296 struct cio2_csi2_timing *timing,
297 unsigned int bpp, unsigned int lanes)
298 {
299 struct device *dev = &cio2->pci_dev->dev;
300 s64 freq;
301 int r;
302
303 if (!q->sensor)
304 return -ENODEV;
305
306 freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
307 if (freq < 0) {
> 308 dev_err(dev, "error %ld, invalid link_freq\n", freq);
309 return freq;
310 }
311
312 timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
313 CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
314 freq,
315 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
316 timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
317 CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
318 freq,
319 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
320 timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
321 CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
322 freq,
323 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
324 timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
325 CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
326 freq,
327 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
328
329 dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
330 dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
331 dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
332 dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
333
334 return 0;
335 };
336
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33706 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ipu3-cio2: Use v4l2_get_link_freq helper
@ 2020-10-15 22:47 ` kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-10-15 22:47 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4314 bytes --]
Hi Sakari,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20201015]
[cannot apply to v5.9]
[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]
url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a012-20201015 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/cf4ab5e39eb042b02f1d5660b5cbd88197a05520
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sakari-Ailus/Link-frequency-helper-for-receivers/20201013-233742
git checkout cf4ab5e39eb042b02f1d5660b5cbd88197a05520
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/pci/intel/ipu3/ipu3-cio2.c:308:50: warning: format specifies type 'long' but the argument has type 's64' (aka 'long long') [-Wformat]
dev_err(dev, "error %ld, invalid link_freq\n", freq);
~~~ ^~~~
%lld
include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
_dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
drivers/media/pci/intel/ipu3/ipu3-cio2.c:301:6: warning: unused variable 'r' [-Wunused-variable]
int r;
^
2 warnings generated.
vim +308 drivers/media/pci/intel/ipu3/ipu3-cio2.c
293
294 /* Calculate the the delay value for termination enable of clock lane HS Rx */
295 static int cio2_csi2_calc_timing(struct cio2_device *cio2, struct cio2_queue *q,
296 struct cio2_csi2_timing *timing,
297 unsigned int bpp, unsigned int lanes)
298 {
299 struct device *dev = &cio2->pci_dev->dev;
300 s64 freq;
301 int r;
302
303 if (!q->sensor)
304 return -ENODEV;
305
306 freq = v4l2_get_link_rate(q->sensor->ctrl_handler, bpp, lanes);
307 if (freq < 0) {
> 308 dev_err(dev, "error %ld, invalid link_freq\n", freq);
309 return freq;
310 }
311
312 timing->clk_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_A,
313 CIO2_CSIRX_DLY_CNT_TERMEN_CLANE_B,
314 freq,
315 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
316 timing->clk_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_A,
317 CIO2_CSIRX_DLY_CNT_SETTLE_CLANE_B,
318 freq,
319 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
320 timing->dat_termen = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_A,
321 CIO2_CSIRX_DLY_CNT_TERMEN_DLANE_B,
322 freq,
323 CIO2_CSIRX_DLY_CNT_TERMEN_DEFAULT);
324 timing->dat_settle = cio2_rx_timing(CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_A,
325 CIO2_CSIRX_DLY_CNT_SETTLE_DLANE_B,
326 freq,
327 CIO2_CSIRX_DLY_CNT_SETTLE_DEFAULT);
328
329 dev_dbg(dev, "freq ct value is %d\n", timing->clk_termen);
330 dev_dbg(dev, "freq cs value is %d\n", timing->clk_settle);
331 dev_dbg(dev, "freq dt value is %d\n", timing->dat_termen);
332 dev_dbg(dev, "freq ds value is %d\n", timing->dat_settle);
333
334 return 0;
335 };
336
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33706 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread