Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: video: imx219: initial driver support #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alanridel
Copy link

@alanridel alanridel commented Aug 6, 2024

initial driver support for IMX219

@alanridel alanridel force-pushed the pr-imx219-pca9542a-driver branch 5 times, most recently from 0aec990 to 5e20aa3 Compare August 12, 2024 09:47
@alanridel alanridel changed the title imx219 driver support drivers: video: imx219: initial driver support Aug 12, 2024
Sony imx219 8 Mega-Pixel CMOS Image Sensor driver support

Signed-off-by: Alan Shaju <[email protected]>
@alanridel alanridel force-pushed the pr-imx219-pca9542a-driver branch from 5e20aa3 to 650f398 Compare August 12, 2024 09:53
Copy link
Collaborator

@josuah josuah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few cosmetic details left, to reduce the work of upstream Zephyr reviewers.
Looks like almost ready!

fmt.pixelformat = VIDEO_PIX_FMT_BGGR8;
fmt.width = 1920;
fmt.height = 1080;
fmt.pitch = fmt.width * 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pitch does not need * 2 in the end: it is a bayer format, there are 1920 pixel wide, and 1920 bytes per row =1 byte per pixel.

@@ -0,0 +1,5 @@
description: IMX219 8 Mega-Pixel CMOS image sensor
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a copyright comment in .yaml files too.

# Copyright (c) 2024 tinyVision.ai Inc.
# SPDX-License-Identifier: Apache-2.0

Comment on lines +1 to +2
/*
* Copyright (c) 2023 Gaurav Singh www.CircuitValley.com
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that all the code got refactored, we can remove this copyright: it is an incompatible license.

@@ -0,0 +1,12 @@
# imx219
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# imx219
# IMX219 8 Mega-Pixel CMOS image sensor

josuah pushed a commit that referenced this pull request Oct 25, 2024
hci_packet_complete(buf, buf_size) should check whether buf_size is
enough.
For instance, hci_packet_complete can receive buf with buf_size 1,
leading to the buffer overflow in cmd->param_len, which is buf[3].
This can happen when rx_thread() receives two frames in 512 bytes
and the first frame size is 511. Then, rx_thread() will call
hci_packet_complete() with 1.

==5==ERROR: AddressSanitizer: global-buffer-overflow on address
0x000000ad81c2 at pc 0x0000005279b3 bp 0x7fffe74f5b70 sp 0x7fffe74f5b68

READ of size 2 at 0x000000ad81c2 thread T6
    #0 0x5279b2  (/root/zephyr.exe+0x5279b2)
    #1 0x4d697d  (/root/zephyr.exe+0x4d697d)
    #2 0x7ffff60e5daa  (/lib/x86_64-linux-gnu/libc.so.6+0x89daa)
(BuildId: 2e01923fea4ad9f7fa50fe24e0f3385a45a6cd1c)

0x000000ad81c2 is located 2 bytes to the right of global variable
'rx_thread.frame' defined in 'zephyr/drivers/bluetooth/hci/userchan.c'
(0xad7fc0) of size 512
SUMMARY: AddressSanitizer: global-buffer-overflow
(/root/zephyr.exe+0x5279b2)
Thread T6 created by T2 here:
    #0 0x48c17c  (/root/zephyr.exe+0x48c17c)
    #1 0x530192  (/root/zephyr.exe+0x530192)
    #2 0x4dcc22  (/root/zephyr.exe+0x4dcc22)

Thread T2 created by T1 here:
    #0 0x48c17c  (/root/zephyr.exe+0x48c17c)
    #1 0x530192  (/root/zephyr.exe+0x530192)
    #2 0x4dcc22  (/root/zephyr.exe+0x4dcc22)

Thread T1 created by T0 here:
    #0 0x48c17c  (/root/zephyr.exe+0x48c17c)
    #1 0x52f36c  (/root/zephyr.exe+0x52f36c)
    #2 0x5371dc  (/root/zephyr.exe+0x5371dc)
    zephyrproject-rtos#3 0x5312a6  (/root/zephyr.exe+0x5312a6)
    zephyrproject-rtos#4 0x52ed7b  (/root/zephyr.exe+0x52ed7b)
    zephyrproject-rtos#5 0x52eddd  (/root/zephyr.exe+0x52eddd)
    zephyrproject-rtos#6 0x7ffff6083c89  (/lib/x86_64-linux-gnu/libc.so.6+0x27c89)
(BuildId: 2e01923fea4ad9f7fa50fe24e0f3385a45a6cd1c)

==5==ABORTING

Signed-off-by: Sungwoo Kim <[email protected]>
josuah pushed a commit that referenced this pull request Nov 12, 2024
With introduction of Raw modes, nRF70 driver now advertises get_c
onfig OP, but doesn't implement all types.

This causes problems two-fold with checksum calculations:
  1. The "config" isn't uninitialized, so, every call returns differnet
     values. So, for UDP header checksum would be done and
     pkt->chksumdone would be set. But for IPv4 header checksum might be
     skipped.
  2. Even if we initialize to zero, then network stack gets all zeros
     and calculates checksum by itself rendering offload moot.

There is another problem in #1, as there is only single flag for pkt for
all checksum, nRF70 driver sees this and tells UMAC to skip checksum for
the entire packet. The design isn't coherent, and should be converted to
communicate per-type checksum status (some are filled by network stack
and some HW).

But as nRF70 support all checksum offloads, advertise all types for both
RX and TX.

Upstream PR #: 80882

Signed-off-by: Chaitanya Tata <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants