Andreas,
------------------------------------------------------------------------
BRs,
Bingbu Cao
>-----Original Message-----
>From: Andreas Helbech Kleist <andreaskleist@xxxxxxxxx>
>Sent: Wednesday, November 8, 2023 7:26 PM
>To: Cao, Bingbu <bingbu.cao@xxxxxxxxx>; linux-media@xxxxxxxxxxxxxxx;
>sakari.ailus@xxxxxxxxxxxxxxx; laurent.pinchart@xxxxxxxxxxxxxxxx
>Cc: andriy.shevchenko@xxxxxxxxxxxxxxx; hdegoede@xxxxxxxxxx;
>ilpo.jarvinen@xxxxxxxxxxxxxxx; claus.stovgaard@xxxxxxxxx;
>tfiga@xxxxxxxxxxxx; senozhatsky@xxxxxxxxxxxx;
>tomi.valkeinen@xxxxxxxxxxxxxxxx; bingbu.cao@xxxxxxxxxxxxxxx; Qiu, Tian Shu
><tian.shu.qiu@xxxxxxxxx>; Wang, Hongju <hongju.wang@xxxxxxxxx>
>Subject: Re: [PATCH v2 08/15] media: intel/ipu6: add IPU6 CSI2 receiver
>v4l2 sub-device
>
>Hi Bingbu,
>
>On Tue, 2023-10-24 at 19:29 +0800, bingbu.cao@xxxxxxxxx wrote:
>> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>>
>> Input system CSI2 receiver is exposed as a v4l2 sub-device.
>> Each CSI2 sub-device represent one single CSI2 hardware port which be
>> linked with external sub-device such camera sensor by linked with ISYS
>> CSI2's sink pad. CSI2 source pad is linked to the sink pad of video
>> capture device.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>> ---
>> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 654
>> ++++++++++++++++++
>> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h | 81 +++
>> .../media/pci/intel/ipu6/ipu6-isys-subdev.c | 379 ++++++++++
>> .../media/pci/intel/ipu6/ipu6-isys-subdev.h | 58 ++
>> .../intel/ipu6/ipu6-platform-isys-csi2-reg.h | 187 +++++
>> 5 files changed, 1359 insertions(+)
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-isys-
>> csi2-reg.h
>>
>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
>> b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
>> new file mode 100644
>> index 000000000000..0dedf3f51850
>> --- /dev/null
>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
>...
>>
>> +
>> +#define DIV_SHIFT 8
>> +#define CSI2_ACCINV 8
>> +
>> +static u32 calc_timing(s32 a, s32 b, s64 link_freq, u32 accinv) {
>> + return accinv * a + (accinv * b * (500000000 >> DIV_SHIFT)
>> + / (int32_t)(link_freq >> DIV_SHIFT)); }
>
>I think accinv should be s32 here. When accinv is unsigned, the expression
>(accinv * b) becomes unsigned, and thus negative values of b gives
>incorrect results.
accinv is always an unsigned value, so I think we don't need change the
type of argument. Following usual arithmetic conversions, I think it needs
a signed integer cast here. What do you think?
>
>This is different from e.g. https://github.com/intel/ipu6-drivers.
>
>/Andreas
